Re: [PATCH v8 01/15] x86/boot: Place kernel_info at a fixed offset
On Thu, Feb 15, 2024 at 08:56:25AM +0100, Ard Biesheuvel wrote: > On Wed, 14 Feb 2024 at 23:31, Ross Philipson > wrote: > > > > From: Arvind Sankar > > > > There are use cases for storing the offset of a symbol in kernel_info. > > For example, the trenchboot series [0] needs to store the offset of the > > Measured Launch Environment header in kernel_info. > > > > Why? Is this information consumed by the bootloader? The bootloader stuffs this info, plus some offset IIRC, into special structure and finally it is consumed by SINIT ACM after GETSEC[SENTER] call. Sadly this data is Intel specific and it is even not compatible with AMD. So, if I am not mistaken, we will need additional member for the AMD in the kernel_info. > I'd like to get away from x86 specific hacks for boot code and boot > images, so I would like to explore if we can avoid kernel_info, or at > least expose it in a generic way. We might just add a 32-bit offset > somewhere in the first 64 bytes of the bootable image: this could > co-exist with EFI bootable images, and can be implemented on arm64, > RISC-V and LoongArch as well. The other architectures may or may not have need for such data due to differences in DRTM implementation. Anyway, whatever we do I want to be sure the DRTM can be used on UEFI and non-UEFI platforms. So, I am not entirely convinced the address/pointer to additional DRTM data should be part of the MS-DOS and/or PE header. Though I am not against building something generic shared among various architectures either. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC v1] kexec: Prototype for signature verification within Xen
On Tue, Jan 15, 2019 at 03:15:28PM +0100, Simon Horman wrote: > On Mon, Jan 14, 2019 at 01:52:07PM -0600, Eric DeVolder wrote: > > These changes work in conjunction with the signature > > verification support for Xen I published recently. > > > > Prior to this change, kexec supported the following > > three modes of operation: > > > > kexec_load: > > - unverified loading of kernel into Linux (original mode) > > - unverified loading of kernel into Xen > > kexec_file_load (the -s option to kexec): > > - verified loading of kernel into Linux > > > > With the verified loading of a kernel into Linux, the scope > > of kexec changed drastically as the kernel performs most of > > the work that kexec previously did; the kernel does so so as > > to reduce the risk of compromise. > > > > For example, the unverified loading of a kernel into Linux > > involves locating memory within the system to load the > > various pieces of data (kernel, initramdisk, command line) > > as well as reserving additional memory such as the first 1MB > > on x86 for legacy reasons as well as something known as > > 'purgatory', a trampoline that checks the integrity of the > > contents of loaded pieces of data, before invoking that > > loaded kernel. The management of purgatory involves > > manipulating an embedded ELF purgatory object file to insert > > a memory hash value, and rewrite a few run-time switches > > based on kexec command line parameters. > > > > By contrast, the verified loading essentially just passes > > file handles for the kernel, initramdisk, and command line > > pointer, and the kernel takes care of the rest, by > > performing all the work that the unverified kexec load would > > do, but inside the kernel using trusted kernel code. > > > > This changeset adds a fourth mode to kexec: > > > > - verified loading of kernel into Xen > > > > In general, Xen performs the signature verification on the > > loaded kernel, much as Linux does, but that is where the > > similarities end. In the current Xen implementation, no > > infrastructure is present to support reading from [Linux > > dom0] file handles, or for manipulating ELF objects. As > > such, without Xen support for these actions, Xen relies upon > > kexec to provide these services, which is what this mode > > does. > > > > To achieve this, this mode of operation essentially vectors > > the verified load for Xen through the non-verified path, > > which performs all the needed actions for kexec to work, but > > then makes an adjustment to pass the entire kernel file, not > > just the loadable portion of the kernel file, to Xen in > > order to provide the proper image for signature > > verification. > > > > The loading of kexec images for signature verification for > > Xen is indicated with the -s switch, just like for Linux. > > > > Changes to configure.ac are for detecting whether or not the > > Xen version supports this kexec_file_load hypercall op. > > > > Changes to kexec-bzImage64.c are for recording what the > > change to the kernel image entry needs to be (the entire > > kernel file, not just the loadable portion), as well as > > vectoring kexec_file_load through kexec_load for Xen. > > > > Changes to kexec-xen.c are to invoke the new Xen > > kexec_file_load hypercall op, from kexec_load. > > > > Changes to kexec.c are to vector kexec_file_load for Xen > > throgh kexec_load for Xen, as well as make the correction > > for passing the complete kernel file to Xen. > > > > Signed-off-by: Eric DeVolder > > Thanks Eric, > > this looks good to me, aside from one nit below. Please do not apply this patch. This is an RFC. Eric, thank you for doing this work. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Xen-devel] RFC Xen signature verification for kexec
On Tue, Apr 24, 2018 at 10:46:38AM +0100, George Dunlap wrote: > On Mon, Apr 23, 2018 at 11:33 AM, Jan Beulich wrote: > On 23.04.18 at 12:25, wrote: > >> On Mon, Apr 23, 2018 at 12:55:45AM -0600, Jan Beulich wrote: > >>> >>> On 20.04.18 at 21:12, wrote: > >>> > Two options for signature verification in Xen > >>> > > >>> > This proposal outlines two options under consideration for enhancing > >>> > Xen to support signature verification of kexec loaded images. The > >>> > first option is essentially to mirror Linux signature verification > >>> > code into Xen. The second option utilizes components from sources > >>> > other than Linux (for example, libgcrypt rather than linux/crypto). > >>> > > >>> > NOTE: An option to utilize dom0 kernel signature verification does not > >>> > prevent the exploit as user space can invoke the hypercall directly, > >>> > bypassing dom0. > >>> > >>> Not exactly - this option nevertheless exists, albeit is perhaps > >>> unattractive: No user space component can issue hypercalls > >>> directly, they always go through the privcmd driver. Hence the > >>> driver cold snoop the kexec hypercall. > >> > >> Hmmm... Is not it a problem from security point of view for us in this > >> case? It should not if dom0 kernel is signed. It have to be signed here. > >> Just thinking a loud... > > > > I'm afraid I don't understand: If the Dom0 kernel isn't signed (or hasn't > > been verified), the system is insecure in the first place. No reason to > > bother measuring the kexec kernel then. > > I think you're both saying the same thing. > > FWIW I wouldn't mind coming up with a hypercall that the privcmd > driver refuses to pass-through as-is, and having some way for the > tools to ask the kernel to check the signature. I have a feeling that I should reformulate the question: How far the Xen hypervisor trusts the privcmd driver? If the privcmd driver is signed then at first sight there should not be a problem. However, we can be more strict and require that (every? Daniel is running away...) hypercall from privcmd to Xen should be verified somehow. Maybe I am overzealous but I think that it make sense to discuss this now than later have problems. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Xen-devel] RFC Xen signature verification for kexec
On Mon, Apr 23, 2018 at 12:55:45AM -0600, Jan Beulich wrote: > >>> On 20.04.18 at 21:12, wrote: > > Two options for signature verification in Xen > > > > This proposal outlines two options under consideration for enhancing > > Xen to support signature verification of kexec loaded images. The > > first option is essentially to mirror Linux signature verification > > code into Xen. The second option utilizes components from sources > > other than Linux (for example, libgcrypt rather than linux/crypto). > > > > NOTE: An option to utilize dom0 kernel signature verification does not > > prevent the exploit as user space can invoke the hypercall directly, > > bypassing dom0. > > Not exactly - this option nevertheless exists, albeit is perhaps > unattractive: No user space component can issue hypercalls > directly, they always go through the privcmd driver. Hence the > driver cold snoop the kexec hypercall. Hmmm... Is not it a problem from security point of view for us in this case? It should not if dom0 kernel is signed. It have to be signed here. Just thinking a loud... > > # > > Option 2: Enable signature verification in Xen utilizing libgcrypt > > > > This option is similar to Option 1, but utilizes libgcrypt > > crytpo library rather than linux/crypto files. > > > > Pros: > > - Libgcrypt is LGPLv2.1+ license. > > - Eliminates problematic scenario of tracking changes to > >linux/crypto sources in Xen, and vice versa in Linux. > > As an initial reaction, of the two options presented I'd prefer this > 2nd one, for this specific reason. > > > Cons: > > - Introduces a dependency on libgcrypt > > - Still relying on lifting many Linux kernel sources for PE file > >handling, certificate handling, etc. However, an alternative > >source for PE file handling is shim. > > That's under the assumption that PE files are the only containers usable > to carry certificates, which I consider odd, not the least because Linux > kernel modules aren't PE either. If the kernel was carrying its certificate I do not think that we care about kernel modules format here... > in a way (also) accessible without parsing PE structures, this dependency > could be dropped. IIRC kexec-tools feeds one file with signature attached into Linux kernel via syscall. It does not support detached signatures. Does it? Eric, could you check it? If it does not then we have to add some code into kexec-tools too. However, I do not think that gain in Xen code will be significant. PE file parsing in this case should be simple. The largest part still would be PKCS#7 and/or PEM parser. Hmmm... There is a chance that there is a lib floating around which does that. Eric, please double check it. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4] kexec-tools: Make xc_dlhandle static
On Thu, Jan 25, 2018 at 09:06:52AM +0100, Simon Horman wrote: > On Wed, Jan 24, 2018 at 08:26:32PM +0100, Daniel Kiper wrote: > > On Wed, Jan 24, 2018 at 01:20:21PM -0600, Eric DeVolder wrote: > > > This patch is a follow-on to commit 894bea93 "kexec-tools: Perform > > > run-time linking of libxenctrl.so". This patch addresses feedback > > > from Daniel Kiper. > > > > > > This patch implements Daniel's suggestion to make the > > > xc_dlhandle variable static, insert missing 'extern' > > > qualifier for the new __xc() wrappers, and correct some > > > style issues. > > > > > > Signed-off-by: Eric DeVolder > > > > Reviewed-by: Daniel Kiper > > Thanks, applied. Simon, thank you. Eric, thank you for doing the work. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4] kexec-tools: Make xc_dlhandle static
On Wed, Jan 24, 2018 at 01:20:21PM -0600, Eric DeVolder wrote: > This patch is a follow-on to commit 894bea93 "kexec-tools: Perform > run-time linking of libxenctrl.so". This patch addresses feedback > from Daniel Kiper. > > This patch implements Daniel's suggestion to make the > xc_dlhandle variable static, insert missing 'extern' > qualifier for the new __xc() wrappers, and correct some > style issues. > > Signed-off-by: Eric DeVolder Reviewed-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3] kexec-tools: Make xc_dlhandle static
On Wed, Jan 24, 2018 at 12:04:34PM -0600, Eric DeVolder wrote:
> This patch is a follow-on to commit 894bea93 "kexec-tools: Perform
> run-time linking of libxenctrl.so". This patch addresses feedback
> from Daniel Kiper.
>
> This patch implements Daniel's suggestion to make the
> xc_dlhandle variable static.
You should also mention that by the way you are adding missing extern
in this patch. Or do it in separate patch with relevant comment.
> Signed-off-by: Eric DeVolder
> Reviewed-by: Daniel Kiper
> ---
> v1: 23jan2018
> - Implemented feedback from Daniel Kiper
>
> v2: 23jan2018
> - Implemented feedback from Daniel Kiper
> - Broke patch into two
>
> v3: 24jan2018
> - Implemented feedback from Daniel Kiper
> - Added 'extern' to the new declarations in kexec-xen.h
> ---
> kexec/kexec-xen.c | 9 -
> kexec/kexec-xen.h | 13 ++---
> 2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 75f8758..960fa16 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,15 @@
> #include "crashdump.h"
>
> #ifdef CONFIG_LIBXENCTRL_DL
> -void *xc_dlhandle;
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +static void *xc_dlhandle;
> xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
> +
> +void *__xc_dlsym(const char *symbol)
> +{
> + return dlsym(xc_dlhandle, symbol);
> +}
> +
> xc_interface *__xc_interface_open(xentoollog_logger *logger,
> xentoollog_logger *dombuild_logger,
> unsigned open_flags)
> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
> index ffb8743..ef08d2c 100644
> --- a/kexec/kexec-xen.h
> +++ b/kexec/kexec-xen.h
> @@ -6,28 +6,27 @@
>
> #ifdef CONFIG_LIBXENCTRL_DL
> #include
I understand that you need this header but I think that it is really
needed in kexec/kexec-xen.c only. Is not it? If yes please drop it
from here and add to kexec/kexec-xen.c. Just after "#include ".
> -/* The handle from dlopen(), needed by dlsym(), dlclose() */
> -extern void *xc_dlhandle;
> +/* Lookup symbols in libxenctrl.so */
> +extern void *__xc_dlsym(const char *symbol);
>
> /* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
> -xc_interface *__xc_interface_open(xentoollog_logger *logger,
> +extern xc_interface *__xc_interface_open(xentoollog_logger *logger,
> xentoollog_logger *dombuild_logger,
> unsigned open_flags);
Broken variable alignment after extern addition. Please fix this too.
Thanks,
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 2/2] kexec-tools: Make xc_dlhandle static
On Tue, Jan 23, 2018 at 01:12:51PM -0600, Eric DeVolder wrote:
> This patch is a follow-on to commit 43d3932e "kexec-tools: Perform
> run-time linking of libxenctrl.so". This patch addresses feedback
> from Daniel Kiper.
>
> This patch implements Daniel's suggestion to make the
> xc_dlhandle variable static.
>
> Signed-off-by: Eric DeVolder
Just two nitpicks... Otherwise you can add
Reviewed-by: Daniel Kiper
> ---
> v1: 23jan2018
> - Implemented feedback from Daniel Kiper
>
> v2: 23jan2018
> - Implemented feedback from Daniel Kiper
> - Broke patch into two
> ---
> kexec/kexec-xen.c | 9 -
> kexec/kexec-xen.h | 9 -
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 75f8758..960fa16 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,15 @@
> #include "crashdump.h"
>
> #ifdef CONFIG_LIBXENCTRL_DL
> -void *xc_dlhandle;
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +static void *xc_dlhandle;
> xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
> +
> +void *__xc_dlsym(const char *symbol)
> +{
> + return dlsym(xc_dlhandle, symbol);
> +}
> +
> xc_interface *__xc_interface_open(xentoollog_logger *logger,
> xentoollog_logger *dombuild_logger,
> unsigned open_flags)
> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
> index ffb8743..8955334 100644
> --- a/kexec/kexec-xen.h
> +++ b/kexec/kexec-xen.h
> @@ -6,9 +6,8 @@
>
> #ifdef CONFIG_LIBXENCTRL_DL
> #include
I think that you can drop this include too.
> -
> -/* The handle from dlopen(), needed by dlsym(), dlclose() */
> -extern void *xc_dlhandle;
> +/* Lookup symbols in libxenctrl.so */
> +void *__xc_dlsym(const char *symbol);
extern void *__xc_dlsym(const char *symbol);?
But then, IMO, you should put extern for __xc_interface_open()
and __xc_interface_close() below too. You can do it in separate
patch with Reviewed-by: Daniel Kiper
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 1/2] kexec-tools: Call dlclose() from within __xc_interface_close()
On Tue, Jan 23, 2018 at 01:12:50PM -0600, Eric DeVolder wrote: > This patch is a follow-on to commit 43d3932e "kexec-tools: Perform > run-time linking of libxenctrl.so". This patch addresses feedback > from Daniel Kiper. > > In the original patch, the call to dlclose() was omitted, in contrast > to the description in the commit message. This patch inserts the call. > Note that this dynamic linking feature is dependent upon the proper > operation of the RTLD_NODELETE flag to dlopen(), which does work as > advertised on Linux (otherwise the call to dlclose() should be omitted). > > Signed-off-by: Eric DeVolder Reviewed-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1] kexec-tools: Tweak run-time handling of libxenctrl.so
Hi Eric,
On Tue, Jan 23, 2018 at 11:39:29AM -0600, Eric DeVolder wrote:
> This patch is a follow-on to commit 43d3932e "kexec-tools: Perform
> run-time linking of libxenctrl.so". This patch addresses feedback
> from Daniel Kiper concerning the lack of a dlclose() and the desire
> to make xc_dlhandle static, rather than extern.
>
> In the original patch, the call to dlclose() was omitted, in contrast
> to the description in the commit message. This patch inserts the call.
> Note that this dynamic linking feature is dependent upon the proper
> operation of the RTLD_NODELETE flag to dlopen(), which does work as
> advertised on Linux (otherwise the call to dlclose() should be omitted).
>
> This patch also implements Daniel's suggestion to make the
> xc_dlhandle variable static.
>
> This patch has been re-tested per scenarios outlined in the original
> patch commit message.
Please split this patch into two separate patches. One for dlclose() change
and one for static xc_dlhandle change. This will help people who will be
doing git bisect in the future. In general it is much appreciated to
provide one logical change per patch.
> Signed-off-by: Eric DeVolder
> ---
> v1: 23jan2018
> - Implemented feedback from Daniel Kiper
> ---
> kexec/kexec-xen.c | 13 -
> kexec/kexec-xen.h | 9 -
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index d42a45a..03ea4a8 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,18 @@
> #include "crashdump.h"
>
> #ifdef CONFIG_LIBXENCTRL_DL
> -void *xc_dlhandle;
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +static void *xc_dlhandle;
> xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
> +
> +void *__xc_dlsym(const char *symbol)
> +{
> + if (xc_dlhandle)
> + return dlsym(xc_dlhandle, symbol);
> + else
> + return NULL;
I am not sure why you need this "if (xc_dlhandle)...". I think that
"return dlsym(xc_dlhandle, symbol);" alone is sufficient here.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4] kexec-tools: Perform run-time linking of libxenctrl.so
On Wed, Jan 17, 2018 at 10:39:01AM -0600, Eric DeVolder wrote:
> When kexec is utilized in a Xen environment, it has an explicit
> run-time dependency on libxenctrl.so. This dependency occurs
> during the configure stage and when building kexec-tools.
[...]
As I saw Simon applied the patch. Great! Sadly I have just
realized that I have missed two things. Please look below.
> The patch creates a wrapper function around xc_interface_open()
> and xc_interface_close() to perform the dlopen() and dlclose().
You say about dlclose() here...
diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
new file mode 100644
index 000..ffb8743
--- /dev/null
+++ b/kexec/kexec-xen.h
[...]
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +extern void *xc_dlhandle;
...and here but it is never called. Is it by design or by mistake?
If by design please add a comment saying why into relevant place,
e.g. __xc_interface_close()? If by mistake please fix it.
[...]
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +extern void *xc_dlhandle;
I am still not happy with xc_dlhandle being extern. IMO it should be static.
I have missed that during last review. Sorry about that. I have a feeling
that you can easily fix it.
Define __xc_dlsym() in kexec/kexec-xen.c just before __xc_interface_open() like
below:
void *__xc_dlsym(const char *symbol)
{
return dlsym(xc_dlhandle, symbol);
}
> +/* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
> +xc_interface *__xc_interface_open(xentoollog_logger *logger,
> + xentoollog_logger *dombuild_logger,
> + unsigned open_flags);
> +int __xc_interface_close(xc_interface *xch);
> +
> +/* GCC expression statements for evaluating dlsym() */
> +#define __xc_call(dtype, name, args...) \
> +( \
> + { dtype value; \
> + typedef dtype (*func_t)(xc_interface *, ...); \
> + func_t func = dlsym(xc_dlhandle, #name); \
And then replace dlsym(xc_dlhandle, #name) with __xc_dlsym(#name)...
> + value = func(args); \
> + value; } \
> +)
> +#define __xc_data(dtype, name) \
> +( \
> + { dtype *value = (dtype *)dlsym(xc_dlhandle, #name); value; } \
...and here. Additionally, you can use __xc_dlsym() instead of dlsym()
in other places in your patch. Latter is not a must...
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3] kexec-tools: Perform run-time linking of libxenctrl.so
On Fri, Jan 12, 2018 at 03:21:13PM -0600, Eric DeVolder wrote: > When kexec is utilized in a Xen environment, it has an explicit > run-time dependency on libxenctrl.so. This dependency occurs > during the configure stage and when building kexec-tools. > > When kexec is utilized in a non-Xen environment (either bare > metal or KVM), the configure and build of kexec-tools omits > any reference to libxenctrl.so. [...] Wow! What a long story! Patch looks quite good. Just a few nitpicks. If you fix them then you can add Reviewed-by: Daniel Kiper to the next version. > Signed-off-by: Eric DeVolder > --- > v1: 29nov2017 > - Daniel Kiper suggested Debian's libxen package of libraries, >but I did not find similar package on most other systems. > > v2: 14dec2017 > - Reposted to kexec and xen-devel mailing lists > > v3: 12jan2018 > - Incorporated feedback from Daniel Kiper. >Configure changes for --with-xen=dl, style problems corrected. >Extensive testing of the new mode. > --- > configure.ac | 27 ++ > kexec/Makefile | 1 + > kexec/arch/i386/crashdump-x86.c| 4 +-- > kexec/arch/i386/kexec-x86-common.c | 4 +-- > kexec/crashdump-xen.c | 4 +-- > kexec/kexec-xen.c | 41 - > kexec/kexec-xen.h | 73 > ++ > 7 files changed, 138 insertions(+), 16 deletions(-) > create mode 100644 kexec/kexec-xen.h > > diff --git a/configure.ac b/configure.ac > index 208dc0a..4dab57f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -167,12 +167,27 @@ if test "$with_xen" = yes ; then > AC_CHECK_HEADER(xenctrl.h, > [AC_CHECK_LIB(xenctrl, xc_kexec_load, , > AC_MSG_NOTICE([Xen support disabled]))]) > - if test "$ac_cv_lib_xenctrl_xc_kexec_load" = 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 Link libxenctrl.so at run-time rather than build-time > +if test "$with_xen" = dl ; then > + AC_CHECK_HEADER(dlfcn.h, > + [AC_CHECK_LIB(dl, dlopen, , > + AC_MSG_ERROR([Dynamic library linking not available]))], > + AC_MSG_ERROR([Dynamic library linking not available])) I think that this error message should be like this: Dynamic library linking header not available > + AC_DEFINE(CONFIG_LIBXENCTRL_DL, 1, [Define to 1 to link libxenctrl.so > at run-time rather than build-time]) > + AC_CHECK_HEADER(xenctrl.h, > + [AC_CHECK_LIB(xenctrl, xc_kexec_load, > + AC_DEFINE(HAVE_LIBXENCTRL, 1, ), # required define, and prevent > -lxenctrl > + AC_MSG_NOTICE([Xen support disabled]))]) > +fi > + > +dnl Check for the Xen kexec_status hypercall - reachable from > --with-xen=yes|dl > +if test "$ac_cv_lib_xenctrl_xc_kexec_load" = 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 > > dnl ---Sanity checks > diff --git a/kexec/Makefile b/kexec/Makefile > index 2b4fb3d..8871731 100644 > --- a/kexec/Makefile > +++ b/kexec/Makefile > @@ -36,6 +36,7 @@ dist += kexec/Makefile > \ > kexec/kexec-elf-boot.h \ > kexec/kexec-elf.h kexec/kexec-sha256.h \ > kexec/kexec-zlib.h kexec/kexec-lzma.h \ > + kexec/kexec-xen.h > \ Could you fix backslash alignment? Maybe in separate patch. It can be part of this series. > kexec/kexec-syscall.h kexec/kexec.h kexec/kexec.8 > > dist += kexec/proc_iomem.c > diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c > index 69a063a..a948d9f 100644 > --- a/kexec/arch/i386/crashdump-x86.c > +++ b/kexec/arch/i386/crashdump-x86.c > @@ -44,9 +44,7 @@ > #include "kexec-x86.h" > #include "crashdump-x86.h" > Please remove this blank line... > -#ifdef HAVE_LIBXENCTRL > -#include > -#endif /* HAVE_LIBXENCTRL */ > +#include "../../kexec-xen.h" > ...and this... Same things below... This was done to separ
Re: [PATCH v2] kexec-tools: Perform run-time linking of libxenctrl.so
ent dlsym() of the required member. > > The patch creates a wrapper function around xc_interface_open() > and xc_interface_close() to perform the dlopen() and dlclose(). > > For the remaining xc_ functions, this patch defines a macro > of the same name which performs the dlsym() and then invokes > the function. See the _xc_call() macro for details. > > There was one data item in libxenctrl.so that presented a > unique problem, HYPERCALL_BUFFER_NULL. It was only utilized > once, as > > set_xen_guest_handle(xen_segs[s].buf.h, HYPERCALL_BUFFER_NULL); > > I tried a variety of techniques but could not find a general > macro-type solution without modifying xenctrl.h. So the > solution was to declare a local HYPERCALL_BUFFER_NULL, and > this appears to work. I admit I am not familiar with libxenctrl > to state if this is a satisfactory workaround, so feedback > here welcome. I can state that this allows kexec to load/unload/kexec > on Xen and non-Xen environments that I've tested without issue. > > With this patch applied, kexec-tools can be built with Xen > support and yet there is no explicit run-time dependency on > libxenctrl.so. Thus it can also be deployed in non-Xen > environments where libxenctrl.so is not installed. > > # ldd build/sbin/kexec > linux-vdso.so.1 => (0x7fff7dbcd000) > liblzma.so.0 => /usr/lib64/liblzma.so.0 (0x0038d900) > libz.so.1 => /lib64/libz.so.1 (0x0038d6c0) > libdl.so.2 => /lib64/libdl.so.2 (0x0038d640) > libc.so.6 => /lib64/libc.so.6 (0x0038d600) > libpthread.so.0 => /lib64/libpthread.so.0 (0x0038d680) > /lib64/ld-linux-x86-64.so.2 (0x562dc0c14000) > # build/sbin/kexec -v > kexec-tools 2.0.16 > > Currently this feature is enabled with the following: > > ./configure --with-xen-dl --with-xen=no > > This is a bit clunky. I welcome feedback such as better names > and/or usage of --with, as well as if we might make this feature > the default. I would do it in a bit different way. If somebody specifies --with-xen then kexec-tools should build as usual. However, if somebody specifies with-xen-dl itself or --with-xen-dl and --with-xen then --with-xen-dl should have a precedence. > Signed-off-by: Eric DeVolder > --- > v1: 29nov2017 > - Daniel Kiper suggested Debian's libxen package of libraries, >but I did not find similar package on most other systems. > > v2: 14dec2017 > - Reposted to kexec and xen-devel mailing lists > --- > configure.ac | 18 ++ > kexec/Makefile | 1 + > kexec/arch/i386/crashdump-x86.c| 4 +--- > kexec/arch/i386/kexec-x86-common.c | 4 +--- > kexec/crashdump-xen.c | 4 +--- > kexec/kexec-xen.c | 43 +- > kexec/kexec-xen.h | 48 > ++ > 7 files changed, 112 insertions(+), 10 deletions(-) > create mode 100644 kexec/kexec-xen.h > > diff --git a/configure.ac b/configure.ac > index 208dc0a..4fc8aa0 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -92,6 +92,9 @@ AC_ARG_WITH([lzma], > AC_HELP_STRING([--without-lzma],[disable lzma support]), > AC_ARG_WITH([xen], AC_HELP_STRING([--without-xen], > [disable extended xen support]), [ with_xen="$withval"], [ with_xen=yes > ] ) > > +AC_ARG_WITH([xen-dl], AC_HELP_STRING([--without-xen-dl], > + [link libxenctrl.so at run-time rather than build-time]), [ > with_xen_dl="$withval"], [ with_xen_dl=no ] ) > + > AC_ARG_WITH([booke], > AC_HELP_STRING([--with-booke],[build for booke]), > AC_DEFINE(CONFIG_BOOKE,1, > @@ -174,6 +177,21 @@ if test "$with_xen" = yes ; then > AC_MSG_NOTICE([The kexec_status call is not > available])) > fi > fi > +if test "$with_xen_dl" = yes ; then > + if test "$with_xen" = yes ; then > + AC_MSG_ERROR([Options --with-xen and --with-xen-dl are mutually > exclusive]) > + fi > + AC_DEFINE(CONFIG_LIBXENCTRL_DL, 1, [Define to 1 to link libxenctrl.so > at run-time rather than build-time]) > + AC_CHECK_HEADER(dlfcn.h, , AC_MSG_ERROR([Dynamic library linking not > available])) > + AC_CHECK_LIB(dl, dlopen, , AC_MSG_ERROR([Dynamic library linking not > available])) Please call AC_CHECK_LIB() from AC_CHECK_HEADER(). You can find more details if you take a look at zlib, lzma and even xenctrl config in configure.ac. > + AC_CHECK_HEADER(xenctrl.h, > + AC_DEFINE(HAVE_LIBXENCTRL, 1, [Define to 1 to enable run-t
Re: [PATCH v1] kexec-tools: Perform run-time linking of libxenctrl.so
On Wed, Nov 29, 2017 at 10:45:16AM -0600, Eric DeVolder wrote: > When kexec is utilized in a Xen environment, it has an explicit > run-time dependency on libxenctrl.so. This dependency occurs > during the configure stage and when building kexec-tools. > > When kexec is utilized in a non-Xen environment (either bare > metal or KVM), the configure and build of kexec-tools omits > any reference to libxenctrl.so. > > Thus today it is not currently possible to configure and build > a *single* kexec that will work in *both* Xen and non-Xen > environments, unless the libxenctrl.so is *always* present. > > For example, a kexec configured for Xen in a Xen environment: > > # ldd build/sbin/kexec > linux-vdso.so.1 => (0x7ffdeba5c000) > libxenctrl.so.4.4 => /usr/lib64/libxenctrl.so.4.4 (0x0038d800) > libz.so.1 => /lib64/libz.so.1 (0x0038d6c0) > libc.so.6 => /lib64/libc.so.6 (0x0038d600) > libdl.so.2 => /lib64/libdl.so.2 (0x0038d640) > libpthread.so.0 => /lib64/libpthread.so.0 (0x0038d680) > /lib64/ld-linux-x86-64.so.2 (0x55e9f8c6c000) > # build/sbin/kexec -v > kexec-tools 2.0.16 > > However, the *same* kexec executable fails in a non-Xen environment: > > # copy xen kexec to . > # ldd ./kexec > linux-vdso.so.1 => (0x7fffa9da7000) > libxenctrl.so.4.4 => not found > liblzma.so.0 => /usr/lib64/liblzma.so.0 (0x003014e0) > libz.so.1 => /lib64/libz.so.1 (0x00300ea0) > libc.so.6 => /lib64/libc.so.6 (0x00300de0) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00300e20) > /lib64/ld-linux-x86-64.so.2 (0x558cc786c000) > # ./kexec -v > ./kexec: error while loading shared libraries: > libxenctrl.so.4.4: cannot open shared object file: No such file or directory > > At Oracle we "workaround" this by having two kexec-tools packages, > one for Xen and another for non-Xen environments. At Oracle, the > desire is to offer a single kexec-tools package that works in either > environment. To achieve this, kexec-tools would either have to ship > with libxenctrl.so (which we have deemed as unacceptable), or we can > make kexec perform run-time linking against libxenctrl.so. > > This patch is one possible way to alleviate the explicit run-time > dependency on libxenctrl.so. This implementation utilizes a set of > macros to wrap calls into libxenctrl.so so that the library can > instead be dlopen() and obtain the function via dlsym() and then > make the call. The advantage of this implementation is that it > requires few changes to the existing kexec-tools code. The dis- > advantage is that it uses macros to remap libxenctrl functions > and do work under the hood. > > Another possible implementation worth considering is the approach > taken by libvmi. Reference the following file: > > https://github.com/libvmi/libvmi/blob/master/libvmi/driver/xen/libxc_wrapper.h > > The libxc_wrapper_t structure definition that starts at line ~33 > has members that are function pointers into libxenctrl.so. This > structure is populated once and then later referenced/dereferenced > by the callers of libxenctrl.so members. The advantage of this > implementation is it is more explicit in managing the use of > libxenctrl.so and its versions, but the disadvantage is it would > require touching more of the kexec-tools code. After some thinking it seems to me that we can consider different approach. Could not we pack all basic Xen libraries into small package called, e.g. libxen? E.g. Debian provides libxen-4.8 which contains following files: /usr/lib/x86_64-linux-gnu/libxencall-4.8.so.1 /usr/lib/x86_64-linux-gnu/libxencall-4.8.so.1.0 /usr/lib/x86_64-linux-gnu/libxenctrl-4.8.so /usr/lib/x86_64-linux-gnu/libxenevtchn-4.8.so.1 /usr/lib/x86_64-linux-gnu/libxenevtchn-4.8.so.1.0 /usr/lib/x86_64-linux-gnu/libxenforeignmemory-4.8.so.1 /usr/lib/x86_64-linux-gnu/libxenforeignmemory-4.8.so.1.0 /usr/lib/x86_64-linux-gnu/libxengnttab-4.8.so.1 /usr/lib/x86_64-linux-gnu/libxengnttab-4.8.so.1.1 /usr/lib/x86_64-linux-gnu/libxenguest-4.8.so /usr/lib/x86_64-linux-gnu/libxenlight-4.8.so /usr/lib/x86_64-linux-gnu/libxentoollog-4.8.so.1 /usr/lib/x86_64-linux-gnu/libxentoollog-4.8.so.1.0 /usr/lib/x86_64-linux-gnu/libxlutil-4.8.so /usr/share/bug/libxen-4.8/control /usr/share/doc/libxen-4.8/changelog.Debian.gz /usr/share/doc/libxen-4.8/changelog.gz /usr/share/doc/libxen-4.8/copyright Then we do not have to install all other Xen stuff to make kexec-tools happy on non-Xen machines. And this should also work for other packages requiring above listed Xen libraries. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1] kexec-tools: Perform run-time linking of libxenctrl.so
On Wed, Nov 29, 2017 at 10:45:16AM -0600, Eric DeVolder wrote: > When kexec is utilized in a Xen environment, it has an explicit > run-time dependency on libxenctrl.so. This dependency occurs > during the configure stage and when building kexec-tools. > > When kexec is utilized in a non-Xen environment (either bare > metal or KVM), the configure and build of kexec-tools omits > any reference to libxenctrl.so. > > Thus today it is not currently possible to configure and build > a *single* kexec that will work in *both* Xen and non-Xen > environments, unless the libxenctrl.so is *always* present. [...] I will take a look at this next week. Thanks, Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [makedumpfile PATCH] Wipe excluded pages that are written into ELF dump file
On Mon, Jul 31, 2017 at 06:35:30AM -0700, Eric DeVolder wrote:
> When a page is excluded by any of the existing dump levels,
> that page may still be written to the ELF dump file, depending
> upon the PFN_EXCLUDED mechanism.
>
> The PFN_EXCLUDED mechanism looks for N consecutive "not
> dumpable" pages, and if found, the current ELF segment is
> closed out and a new ELF segment started, at the next dumpable
> page. Otherwise, if the PFN_EXCLUDED criteria is not meet (that
> is, there is a mix of dumpable and not dumpable pages, but not
> N consecutive not dumpable pages) all pages are written to the
> dump file.
>
> This patch implements a mechanism for those "not dumpable" pages
> that are written to the ELF dump file to fill those pages with
> constant data, rather than the original data. In other words,
> the dump file still contains the page, but its data is wiped.
>
> The motivation for doing this is to protect real user (customer)
> data from "leaking" through to a dump file when that data was
> intended to be omitted.
>
> Signed-off-by: Eric DeVolder
> ---
> makedumpfile.8 | 8
> makedumpfile.c | 32 +---
> makedumpfile.h | 2 ++
> 3 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/makedumpfile.8 b/makedumpfile.8
> index f94f2d7..64af0cf 100644
> --- a/makedumpfile.8
> +++ b/makedumpfile.8
> @@ -621,6 +621,14 @@ order from left to right. \fIVMCORE\fRs are assembled
> into a single
> # makedumpfile \-x vmlinux \-\-diskset=vmcore1 \-\-diskset=vmcore2 dumpfile
>
> .TP
> +\fB\-\-fill-excluded-pages FILL_VALUE\fR
I am OK with --fill-excluded-pages to change default value but it is not needed
in my opinion. However, I think that we should have --no-fill-excluded-pages
variant. Just in case if somebody wish to disable default behavior
> +For the ELF dump file format, excluded pages may be written into the dump
> +file, but the page contents are wiped. This option allows the wipe value
> +to be changed from the default 0x5041474557495045UL "PAGEWIPE", or on
> +32-bit systems "WIPE".
0xDEAD9A6E == DEADPAGE where P == 9 (do you have better figure for P?) and G ==
6
This seems better for me because you do not need to convert it to ASCII
to see what it is. And fills exactly 32-bits.
> +
> +
> +.TP
> \fB\-D\fR
> Print debugging message.
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index f85003a..cee0ab0 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -7139,7 +7139,7 @@ out:
>
> int
> write_elf_load_segment(struct cache_data *cd_page, unsigned long long paddr,
> -off_t off_memory, long long size)
> +off_t off_memory, long long size, struct cycle *cycle)
> {
> long page_size = info->page_size;
> long long bufsz_write;
> @@ -7163,10 +7163,23 @@ write_elf_load_segment(struct cache_data *cd_page,
> unsigned long long paddr,
> else
> bufsz_write = size;
>
> - if (read(info->fd_memory, buf, bufsz_write) != bufsz_write) {
> - ERRMSG("Can't read the dump memory(%s). %s\n",
> - info->name_memory, strerror(errno));
> - return FALSE;
> + if (!is_dumpable(info->bitmap2, paddr_to_pfn(paddr), cycle)) {
> + unsigned k;
> + unsigned long *p = (unsigned long *)buf;
> + for (k = 0; k < info->page_size; k += sizeof(unsigned
> long)) {
> + *p++ = info->fill_excluded_pages_value;
> + }
> + if (lseek(info->fd_memory, bufsz_write, SEEK_CUR) < 0) {
> + ERRMSG("Can't seek the dump memory(%s). %s\n",
> + info->name_memory, strerror(errno));
> + return FALSE;
> + }
> + } else {
> + if (read(info->fd_memory, buf, bufsz_write) !=
> bufsz_write) {
> + ERRMSG("Can't read the dump memory(%s). %s\n",
> + info->name_memory, strerror(errno));
> + return FALSE;
> + }
> }
> filter_data_buffer((unsigned char *)buf, paddr, bufsz_write);
> paddr += bufsz_write;
> @@ -7431,7 +7444,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header,
> struct cache_data *cd_page)
>*/
> if (load.p_filesz)
> if (!write_elf_load_segment(cd_page,
> paddr,
> - off_memory,
> load.p_filesz))
> + off_memory,
> load.p_filesz, &cycle))
> return FALSE;
>
> load.p_paddr += load.p_memsz;
> @@ -7473,7 +7486
Re: [makedumpfile PATCH v2] Prevent data loss in last page of ELF core dumpfile
On Wed, Jul 05, 2017 at 12:13:22PM -0500, Eric DeVolder wrote: > When generating an ELF core dump file, if a segment size > is not an exact multiple of PAGE_SIZE, then the corresponding > generated segment is erroneously truncated to a PAGE_SIZE multiple. > Thus a small loss of data up to PAGE_SIZE-1 bytes can occur. Some nit picks below... [...] > With the patch applied, the file size is again correct. > > Signed-off-by: Eric DeVolder > --- > v2: posted 05jul2017 to kexec-tools list > - feedback from Atsushi Kumagai pointed to real root of problem, >and patch changed accordingly > > v1: posted 03jul2017 to kexec-tools list > --- > makedumpfile.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/makedumpfile.c b/makedumpfile.c > index e69b6df..26296f1 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -5410,7 +5410,8 @@ create_1st_bitmap_file(void) > if (pfn_start > info->max_mapnr) > continue; > pfn_end = MIN(pfn_end, info->max_mapnr); > - > +/* Account for last page if it has less than page_size data in it */ > +if (phys_end & (info->page_size-1)) ++pfn_end; Something is wrong with column alignment. Tabs instead of spaces? And "++pfn_end;" should be in new line. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: makedumpfile PATCH] Fix the use of Xen physical and machine addresses
On Tue, May 30, 2017 at 11:28:15PM +0200, Petr Tesarik wrote: > On Tue, 30 May 2017 14:30:43 -0500 > Eric DeVolder wrote: > > > Hi, > > Testing is underway. Generally working but I do have a couple of fails > > I'm looking into: > > > > > FAIL xen-4.1.2-rc3-pre_domU-pv_xl_linux-2.6.39-x86_64.tar.bz2 > ^^^ > > Is this a PV DomU xc_core style dump? If so, then it has never been > supported by makedumpfile, because the file format is very different, > with no program headers, but a special section called ".xen_pages". In > fact, the file format itself does not cover this type of dump. Ahhh... IIRC, you are right. Thanks for pointing this out. Eric, please double check it and if PV __GUESTS__ are not supported by makedumpfile skip tests for them. Just test dom0 and Xen hypervisor stuff. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Crash-utility] [PATCH] xen: Add support for domU with Linux kernel 3.19 and newer
On Wed, May 24, 2017 at 11:56:28AM -0400, Dave Anderson wrote: > - Original Message - > > crash patch c3413456599161cabc4e910a0ae91dfe5eec3c21 (xen: Add support for > > dom0 with Linux kernel 3.19 and newer) from Daniel made crash utility > > support xen dom0 vmcores after linux kernel commit > > 054954eb051f35e74b75a566a96fe756015352c8 (xen: switch to linear virtual > > mapped sparse p2m list). > > > > This patch can be deemed as a subsequent and make this utility support Xen > > PV domU dumpfiles again. > > > > Basically speaking, readmem() can't be used to read xen_p2m_addr associate > > memory directly during m2p translation. It introduces infinite recursion. > > Following call sequence shows the scenario, it comes from a section of > > backtrace with only kvaddr, machine addr and mfn left as parameter: > > > > module_init() > > > > /* The first readmem() from module_init(). */ > > readmem(addr=0xa02fe4a0) > > > > /* readmem() needs physical address, so calls kvtop(). */ > > kvtop(kvaddr=0xa02fe4a0) > > x86_64_kvtop(kvaddr=a02fe4a0) > > > > /* Calculate physical address by traversing page tables. */ > > x86_64_kvtop_xen_wpt(kvaddr=0xa02fe4a0) > > > > /* > > * x86_64_kvtop_xen_wpt() is going to traverse the page table to > > * get the physical address for 0xa02fe4a0. So, at first it > > * is needed to translate the pgd from machine address to physical > > * address. So invoke xen_m2p() here to do the translation. 0x58687f000 > > * is the pgd machine address in x86_64_kvtop_xen_wpt() and is needed > > * to be translated to its physical address. > > */ > > xen_m2p(machine=0x58687f000) > > __xen_m2p(machine=0x58687f000, mfn=0x58687f) > > > > /* > > * __xen_m2p() is going to search mfn 0x58687f in p2m VMA which starts > > * at VMA 0xc91cf000. It compares every mfn stored in it with > > * 0x58687f. Once it's proved 0x58687f is one mfn in the p2m, its offset > > * will be used to calculate the pfn. > > * > > * readmem() is invoked by __xen_m2p() to read the page from VMA > > * 0xc91cf000 here. > > */ > > readmem(addr=0xc91cf000) > > > > /* > > * readmem() needs physical address of 0xc91cf000 to make the > > * reading done. So it invokes kvtop() to get the physical address. > > */ > > kvtop(kvaddr=0xc91cf000) > > x86_64_kvtop(kvaddr=0xc91cf000) > > > > /* It needs to calculate physical address by traversing page tables. */ > > x86_64_kvtop_xen_wpt(kvaddr=0xc91cf000) > > > > /* > > * 0x581b7e000 is the machine address of pgd need to be translated here. > > * The mfn is calculated in this way at x86_64_kvtop_xen_wpt(): > > * > > * pml4 = ((ulong *)machdep->machspec->pml4) + pml4_index(kvaddr); > > * pgd_paddr = (*pml4) & PHYSICAL_PAGE_MASK; > > * > > * The kvaddr 0xc91cf000 here is quite different from the one > > * above, so the machine address of pgd is not the same one. And this > > * pgd is the one we use to access the VMA of p2m table. > > */ > > xen_m2p(machine=0x581b7e000) > > __xen_m2p(machine=0x581b7e000, mfn=0x581b7e) > > > > /* > > * Looking for mfn 0x581b7e in the range of p2m page which starts at > > * VMA 0xc91f5000. > > */ > > readmem(addr=0xc91f5000) > > > > /* Need physical address of VMA 0xc91f5000 as same reason above. */ > > kvtop(kvaddr=0xc91f5000) > > x86_64_kvtop(kvaddr=0xc91f5000) > > > > /* Need to traverse page tables to calculate physical address for it. */ > > x86_64_kvtop_xen_wpt(kvaddr=0xc91f5000) > > > > /* > > * Unfortunately, machine address 0x581b7e000 have to be translated again. > > * Endless loop starts from here. > > */ > > xen_m2p(machine=0x581b7e000) > > __xen_m2p(machine=0x581b7e000, mfn=0x581b7e) > > readmem(addr=0xc91f5000) > > > > Fortunately, PV domU p2m mapping is also stored at xd->xfd + > > xch_index_offset > > and organized as struct xen_dumpcore_p2m. We have a chance to read the p2m > > stuff directly from there, and then we avoid the loop above. > > > > So, this patch implements a special reading function read_xc_p2m() to > > extract > > the mfns from xd->xfd + xch_index_offset. This function does not need to > > read > > mfns from p2m VMA like readmem() does, so, we avoid the endless loop > > introduced > > by the address translation. > > > > Signed-off-by: Honglei Wang > > Reviewed-by: Daniel Kiper > > Queued for crash-7.2.0: > > > https://github.com/crash-utility/crash/commit/5c52842a58a2602dba81de71831af98b2b53c6e0 Wow, Dave, you are fast! Thanks a lot! Honglei, congrats! Thanks for doing the work! Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[ptesa...@suse.cz: [makedumpfile PATCH] Fix the use of Xen physical and machine addresses]
Hi Eric,
May I ask you to do the tests of this patch with our (Xen) dumpfiles?
After that please drop the summary of the results here. If you have
any questions drop me a line.
Daniel
- Forwarded message from Petr Tesarik -
Date: Tue, 23 May 2017 07:46:40 +0200
From: Petr Tesarik
To: Atsushi Kumagai
Cc: kexec@lists.infradead.org, Daniel Kiper
Subject: [makedumpfile PATCH] Fix the use of Xen physical and machine addresses
X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.31; x86_64-suse-linux-gnu)
There is currently some support for physical-to-machine translation in
makedumpfile, but it is inconsistent.
Most importantly, vaddr_to_paddr() may return either a physical address
or a machine address:
1. If the return value is calculated directly (by subtracting direct
mapping offset, or by consulting ELF headers), then it is a guest
physical address (as seen by the Linux kernel).
2. If the return value is taken from page tables, then it is a machine
address (as seen by the CPU).
Interestingly, makedumpfile never uses guest physical addresses, except
in __exclude_unnecessary_pages(), which already performs the required
ptom conversion explicitly.
So, the best solution is to treat PADDR as "the address seen by the CPU"
(be it on bare metal or under Xen) and get rid of MADDR_XEN. For addresses
that are translated directly, vaddr_to_paddr() return value must be
converted using ptom_xen(), but this call is in fact merely moved from
readmem().
This patch has been tested on a few bare metal and Xen dumps (x86_64 and
x86).
Signed-off-by: Petr Tesarik
---
arch/ia64.c| 8 +---
arch/x86.c | 19 +--
arch/x86_64.c | 19 ---
makedumpfile.c | 45 -
makedumpfile.h | 2 +-
5 files changed, 39 insertions(+), 54 deletions(-)
diff --git a/arch/ia64.c b/arch/ia64.c
index e629f94..6c33cc7 100644
--- a/arch/ia64.c
+++ b/arch/ia64.c
@@ -243,6 +243,8 @@ vtop_ia64(unsigned long vaddr)
if (!is_vmalloc_addr_ia64(vaddr)) {
paddr = vaddr - info->kernel_start +
(info->phys_base & KERNEL_TR_PAGE_MASK);
+ if (is_xen_memory())
+ paddr = ptom_xen(paddr);
return paddr;
}
@@ -301,7 +303,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
dirp = SYMBOL(frametable_pg_dir) - DIRECTMAP_VIRT_START;
dirp += ((addr >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1)) * sizeof(unsigned
long long);
- if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
+ if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
return NOT_PADDR;
dirp = entry & _PFN_MASK;
@@ -309,7 +311,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
return NOT_PADDR;
dirp += ((addr >> PMD_SHIFT) & (PTRS_PER_PMD - 1)) * sizeof(unsigned
long long);
- if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
+ if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
return NOT_PADDR;
dirp = entry & _PFN_MASK;
@@ -317,7 +319,7 @@ kvtop_xen_ia64(unsigned long kvaddr)
return NOT_PADDR;
dirp += ((addr >> PAGESHIFT()) & (PTRS_PER_PTE - 1)) * sizeof(unsigned
long long);
- if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry)))
+ if (!readmem(PADDR, dirp, &entry, sizeof(entry)))
return NOT_PADDR;
if (!(entry & _PAGE_P))
diff --git a/arch/x86.c b/arch/x86.c
index 1b4d2b6..3fdae93 100644
--- a/arch/x86.c
+++ b/arch/x86.c
@@ -233,8 +233,11 @@ vaddr_to_paddr_x86(unsigned long vaddr)
{
unsigned long long paddr;
- if ((paddr = vtop_x86_remap(vaddr)) != NOT_PADDR)
+ if ((paddr = vtop_x86_remap(vaddr)) != NOT_PADDR) {
+ if (is_xen_memory())
+ paddr = ptom_xen(paddr);
return paddr;
+ }
if ((paddr = vaddr_to_paddr_general(vaddr)) != NOT_PADDR)
return paddr;
@@ -247,8 +250,12 @@ vaddr_to_paddr_x86(unsigned long vaddr)
ERRMSG("Can't get necessary information for vmalloc
translation.\n");
return NOT_PADDR;
}
- if (!is_vmalloc_addr_x86(vaddr))
- return (vaddr - info->kernel_start);
+ if (!is_vmalloc_addr_x86(vaddr)) {
+ paddr = vaddr - info->kernel_start;
+ if (is_xen_memory())
+ paddr = ptom_xen(paddr);
+ return paddr;
+ }
if (vt.mem_flags & MEMORY_X86_PAE) {
paddr = vtop_x86_PAE(vaddr);
@@ -280,7 +287,7 @@ kvtop_xen_x86(unsigned long kvaddr)
if ((dirp = kvtop_xen_x86(SYMBOL(pgd_l3))) == NOT_PADDR)
return NOT_PADDR;
dirp += pgd_index_PAE(kvaddr) * sizeof(unsigned long long);
- if (!readmem(MADDR_XEN, dirp, &entry, sizeof(entry))
Re: [PATCH v0] Fix broken Xen support in configure.ac
On Fri, Apr 07, 2017 at 09:13:06AM -0500, Eric DeVolder wrote: > Commit 2cf7cb9a "kexec: implemented XEN KEXEC STATUS to determine > if an image is loaded" added configure-time detection of the > kexec_status() call, but in doing so had the unintended side > effect of disabling support for Xen altogether due to the > missing HAVE_LIBXENCTRL=1. This corrects the broken behavior > while still maintaining the original intention of detecting > support for kexec_status() call. In general Reviewed-by: Daniel Kiper but two nitpicks. You do not need v0 in subject if you post first patch. Plain "[PATCH]" (of course without quotes) is sufficient. > --- > Broken behavior (HAVE_LIBXENCTRL is missing altogether): > ... > checking xenctrl.h usability... yes > checking xenctrl.h presence... yes > checking for xenctrl.h... yes > checking for xc_kexec_load in -lxenctrl... yes > checking for xc_kexec_status in -lxenctrl... yes > > in include/config.h: > /* The kexec_status call is available */ > #define HAVE_KEXEC_CMD_STATUS 1 > > Fixed behaviour (restores HAVE_LIBXENCTRL): > ... > checking xenctrl.h usability... yes > checking xenctrl.h presence... yes > checking for xenctrl.h... yes > checking for xc_kexec_load in -lxenctrl... yes > checking for xc_kexec_status in -lxenctrl... yes > > in include/config.h: > /* The kexec_status call is available */ > #define HAVE_KEXEC_CMD_STATUS 1 > /* Define to 1 if you have the `xenctrl' library (-lxenctrl). */ > #define HAVE_LIBXENCTRL 1 > > Reported-and-Tested-by: Konrad Rzeszutek Wilk > Signed-off-by: Eric DeVolder Reported-and-Tested-by, Signed-off-by et consortes should be just before first "---". Otherwise "git am" will drop all of them. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] crashdump: Remove stray get_crashkernel_region() declaration
Signed-off-by: Daniel Kiper --- kexec/crashdump.h |1 - 1 file changed, 1 deletion(-) diff --git a/kexec/crashdump.h b/kexec/crashdump.h index 96219a8..86e1ef2 100644 --- a/kexec/crashdump.h +++ b/kexec/crashdump.h @@ -1,7 +1,6 @@ #ifndef CRASHDUMP_H #define CRASHDUMP_H -int get_crashkernel_region(uint64_t *start, uint64_t *end); extern int get_crash_notes_per_cpu(int cpu, uint64_t *addr, uint64_t *len); extern int get_kernel_vmcoreinfo(uint64_t *addr, uint64_t *len); extern int get_xen_vmcoreinfo(uint64_t *addr, uint64_t *len); -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5 00/11] kexec: Add option to get crash kernel region size
On Thu, Mar 02, 2017 at 10:51:51AM +0100, Simon Horman wrote: > On Fri, Feb 17, 2017 at 04:47:14PM -0600, Eric DeVolder wrote: > > This is the fifth version of a patch series originally posted by > > Daniel Kiper on December 5. > > Thanks, applied. Thanks a lot! Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2] gitignore: add two generated files in purgatory
On Tue, Feb 21, 2017 at 10:18:24AM -0600, Eric DeVolder wrote: > This patch adds the two generated files below to .gitignore, > so that 'git status' does not complain about them. This is not a problem. Do you have issues during 'git checkout' or so? If yes then I think it is worth considering. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 11/11] kexec: Add option to get crash kernel region size
On Tue, Feb 14, 2017 at 03:09:23PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Crash kernel region size is available via sysfs on Linux running on
> bare metal. However, this does not work when Linux runs as Xen dom0.
> In this case Xen crash kernel region size should be established using
> __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does
> not make a lot of sense in Xen dom0). Sadly hypercalls are not easily
> accessible using shell scripts or something like that. Potentially we
> can check "xl dmesg" output for crashkernel option but this is not nice.
> So, let's add this functionality, for Linux running on bare metal and
> as Xen dom0, to kexec-tools. This way kdump scripts may establish crash
> kernel region size in one way regardless of platform. All burden of
> platform detection lies on kexec-tools.
>
> Figure (and unit) displayed by this new kexec-tools functionality is
> the same as one taken from /sys/kernel/kexec_crash_size.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> Reviewed-by: Daniel Kiper
> ---
> v4: Incorporated feedback:
> - changes for coding convention and formatting
> - Changed commit description to make it clear that
> get_crash_kernel_load_range() is a stub on some archs
> v3: Incorporated feedback:
> - changes for coding convention and formatting
> - restructured to introduce get_crash_kernel_load_range() for each
> architecture, and then a single function in kexec/kexec.c to call
> the per-architecture get_crash_kernel_load_range() and print the
> result.
> v2: Incorporated feedback:
> - utilize the is_crashkernel_mem_reserved() function common in all archs
> - for ppc and ppc64, utilize device-tree values to print size
> - for unsupported architectures, print appropriate message
> v1: Posted to kexec-tools mailing list
> ---
> kexec/kexec.8 | 3 +++
> kexec/kexec.c | 19 +++
> kexec/kexec.h | 4 +++-
> 3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/kexec/kexec.8 b/kexec/kexec.8
> index f4b39a6..e0131b4 100644
> --- a/kexec/kexec.8
> +++ b/kexec/kexec.8
> @@ -179,6 +179,9 @@ Load a helper image to jump back to original kernel.
> .TP
> .BI \-\-reuseinitrd
> Reuse initrd from first boot.
> +.TP
> +.BI \-\-print-ckr-size
> +Print crash kernel region size, if available.
>
>
> .SH SUPPORTED KERNEL FILE TYPES AND OPTIONS
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a2ba79d..eccd988 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -995,6 +995,7 @@ void usage(void)
> " --mem-max= Specify the highest memory address to\n"
> " load code into.\n"
> " --reuseinitrdReuse initrd from first boot.\n"
> +" --print-ckr-size Print crash kernel region size.\n"
> " --load-preserve-context Load the new kernel and preserve\n"
> " context of current kernel during kexec.\n"
> " --load-jump-back-helper Load a helper image to jump back\n"
> @@ -1218,6 +1219,21 @@ static int do_kexec_file_load(int fileind, int argc,
> char **argv,
> return ret;
> }
>
> +static void print_crashkernel_region_size(void)
> +{
> + uint64_t start = 0, end = 0;
> +
> + if (is_crashkernel_mem_reserved() &&
> + get_crash_kernel_load_range(&start, &end)) {
get_crash_kernel_load_range() call should start in the same
column as is_crashkernel_mem_reserved() call starts.
So, we should have:
if (is_crashkernel_mem_reserved() &&
get_crash_kernel_load_range(&start, &end)) {
> + fprintf(stderr, "get_crash_kernel_load_range()
> failed.\n");
> + return;
Please just only two tabs.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 07/11] crashdump/ppc: Add get_crash_kernel_load_range() function
On Tue, Feb 14, 2017 at 03:09:19PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Implement get_crash_kernel_load_range() in support of
> print crash kernel region size option.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> Reviewed-by: Daniel Kiper
> ---
> v4: Incorporated feedback:
> - changes for coding convention and formatting
> v3: Incorporated feedback:
> - changes for coding convention and formatting
> - restructured to introduce get_crash_kernel_load_range() for each
> architecture, and then a single function in kexec/kexec.c to call
> the per-architecture get_crash_kernel_load_range() and print the
> result.
> - changed print_crashkernel_region_size() to get_crash_kernel_load_range()
> - introduced get_devtree_value()
> v2: Incorporated feedback:
> - utilize the is_crashkernel_mem_reserved() function common in all archs
> - utilize device-tree values to print size
> v1: Posted to kexec-tools mailing list
> ---
> kexec/arch/ppc/crashdump-powerpc.c | 27 ++-
> kexec/arch/ppc/kexec-ppc.c | 28
> kexec/arch/ppc/kexec-ppc.h | 1 +
> 3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/kexec/arch/ppc/crashdump-powerpc.c
> b/kexec/arch/ppc/crashdump-powerpc.c
> index 3dc35eb..186005c 100644
> --- a/kexec/arch/ppc/crashdump-powerpc.c
> +++ b/kexec/arch/ppc/crashdump-powerpc.c
> @@ -16,6 +16,9 @@
> #include "kexec-ppc.h"
> #include "crashdump-powerpc.h"
>
> +#define DEVTREE_CRASHKERNEL_BASE
> "/proc/device-tree/chosen/linux,crashkernel-base"
> +#define DEVTREE_CRASHKERNEL_SIZE
> "/proc/device-tree/chosen/linux,crashkernel-size"
> +
> #ifdef CONFIG_PPC64
> static struct crash_elf_info elf_info64 = {
> class: ELFCLASS64,
> @@ -397,11 +400,33 @@ void add_usable_mem_rgns(unsigned long long base,
> unsigned long long size)
> usablemem_rgns.size, base, size);
> }
>
> +int get_crash_kernel_load_range(uint64_t *start, uint64_t *end)
> +{
> + unsigned long long value;
> + int ret = 0;
> +
> + if (!get_devtree_value(DEVTREE_CRASHKERNEL_BASE, &value))
> + *start = value;
> + else {
> + *start = 0;
I have a feeling that this is not needed. If it is true then we can also
drop curly brackets and leave just ret assignment only.
> + ret = -1;
> + }
> +
> + if (!get_devtree_value(DEVTREE_CRASHKERNEL_SIZE, &value))
> + *end = *start + value - 1;
> + else {
> + *end = 0;
Ditto.
IIRC, same thing applies to ppc64.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] gitignore: add two generated files in purgatory
On Fri, Feb 10, 2017 at 04:20:05PM -0600, Eric DeVolder wrote: > This patch adds the two generated files below to .gitignore. Next time please add why it is needed. One sentence explanation is sufficient. > purgatory/purgatory.map > purgatory/purgatory.ro.sym > > Signed-off-by: Eric DeVolder Otherwise: Reviewed-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 11/11] kexec: Add option to get crash kernel region size
On Fri, Feb 10, 2017 at 04:07:42PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> From: Daniel Kiper
Hmmm... One is sufficient...
> Crash kernel region size is available via sysfs on Linux running on
> bare metal. However, this does not work when Linux runs as Xen dom0.
> In this case Xen crash kernel region size should be established using
> __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does
> not make a lot of sense in Xen dom0). Sadly hypercalls are not easily
> accessible using shell scripts or something like that. Potentially we
> can check "xl dmesg" output for crashkernel option but this is not nice.
> So, let's add this functionality, for Linux running on bare metal and
> as Xen dom0, to kexec-tools. This way kdump scripts may establish crash
> kernel region size in one way regardless of platform. All burden of
> platform detection lies on kexec-tools.
>
> Figure (and unit) displayed by this new kexec-tools functionality is
> the same as one taken from /sys/kernel/kexec_crash_size.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> ---
> kexec/kexec.8 | 3 +++
> kexec/kexec.c | 20
> kexec/kexec.h | 4 +++-
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/kexec/kexec.8 b/kexec/kexec.8
> index f4b39a6..e0131b4 100644
> --- a/kexec/kexec.8
> +++ b/kexec/kexec.8
> @@ -179,6 +179,9 @@ Load a helper image to jump back to original kernel.
> .TP
> .BI \-\-reuseinitrd
> Reuse initrd from first boot.
> +.TP
> +.BI \-\-print-ckr-size
> +Print crash kernel region size, if available.
>
>
> .SH SUPPORTED KERNEL FILE TYPES AND OPTIONS
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a2ba79d..0be1be4 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -995,6 +995,7 @@ void usage(void)
> " --mem-max= Specify the highest memory address to\n"
> " load code into.\n"
> " --reuseinitrdReuse initrd from first boot.\n"
> +" --print-ckr-size Print crash kernel region size.\n"
> " --load-preserve-context Load the new kernel and preserve\n"
> " context of current kernel during kexec.\n"
> " --load-jump-back-helper Load a helper image to jump back\n"
> @@ -1218,6 +1219,22 @@ static int do_kexec_file_load(int fileind, int argc,
> char **argv,
> return ret;
> }
>
> +static void print_crashkernel_region_size(void)
> +{
> + uint64_t start = 0, end = 0;
This initialization is not needed.
> +
> + if (is_crashkernel_mem_reserved()) {
> + int ret = get_crash_kernel_load_range(&start, &end);
I do not like mixing code with variable declarations.
> + if (ret != 0) {
if (ret)
> + fprintf(stderr, "get_crash_kernel_load_range
> failed.\n");
s/get_crash_kernel_load_range/get_crash_kernel_load_range()/
> + return;
> + }
> + }
Please add empty line here.
> + if (start != end)
if (!ret && start != end)
There is no guarantee that in case of error nobody touched start and/or
end in get_crash_kernel_load_range() call.
> + printf("%lu\n", end - start + 1);
> + else
> + printf("0\n");
> +}
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 08/11] crashdump/ppc64: Add get_crash_kernel_load_range() function
On Fri, Feb 10, 2017 at 04:07:39PM -0600, Eric DeVolder wrote: > From: Daniel Kiper > > Implement get_crash_kernel_load_range() in support of > print crash kernel region size option. Most of comments for #07 applies here too. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 07/11] crashdump/ppc: Add get_crash_kernel_load_range() function
On Fri, Feb 10, 2017 at 04:07:38PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Implement get_crash_kernel_load_range() in support of
> print crash kernel region size option.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> ---
> kexec/arch/ppc/crashdump-powerpc.c | 22 +-
> kexec/arch/ppc/kexec-ppc.c | 27 +++
> kexec/arch/ppc/kexec-ppc.h | 1 +
> 3 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/kexec/arch/ppc/crashdump-powerpc.c
> b/kexec/arch/ppc/crashdump-powerpc.c
> index 3dc35eb..fc5d70c 100644
> --- a/kexec/arch/ppc/crashdump-powerpc.c
> +++ b/kexec/arch/ppc/crashdump-powerpc.c
> @@ -397,6 +397,27 @@ void add_usable_mem_rgns(unsigned long long base,
> unsigned long long size)
> usablemem_rgns.size, base, size);
> }
>
> +int get_crash_kernel_load_range(uint64_t *start, uint64_t *end)
> +{
> + unsigned long long value;
> + int ret = 0;
> +
> + if
> (get_devtree_value("/proc/device-tree/chosen/linux,crashkernel-base", &value))
Could not you define this path as a constant somewhere? It is used in a
few places, so, it would be nice.
> + *start = value;
> + else {
> + *start = 0;
> + ret = -1;
> + }
I would add empty line here.
> + if
> (get_devtree_value("/proc/device-tree/chosen/linux,crashkernel-size", &value))
Ditto.
> + *end = *start + value - 1;
> + else {
> + *end = 0;
> + ret = -1;
> + }
> +
> + return ret;
> +}
> +
> int is_crashkernel_mem_reserved(void)
> {
> int fd;
> @@ -407,4 +428,3 @@ int is_crashkernel_mem_reserved(void)
> close(fd);
> return 1;
> }
> -
Nice but this cleanup should be a separate patch if you wish.
Otherwise it is a bit confusing.
> diff --git a/kexec/arch/ppc/kexec-ppc.c b/kexec/arch/ppc/kexec-ppc.c
> index d046110..d9cdb9c 100644
> --- a/kexec/arch/ppc/kexec-ppc.c
> +++ b/kexec/arch/ppc/kexec-ppc.c
> @@ -423,6 +423,33 @@ err_out:
> return -1;
> }
>
> +int get_devtree_value(const char *fname, unsigned long long *pvalue)
> +{
> + /* Return 1 if fname/value valid, 0 otherwise */
Most if not all functions in kexec/arch/ppc/kexec-ppc.c return -1 in case
of error and 0 if everything went OK. Please follow this convention.
> + FILE *file;
> + char buf[MAXBYTES];
> + int ret = 1, n = -1;
> + unsigned long long value = 0;
> +
> + if ((file = fopen(fname, "r"))) {
> + n = fread(buf, 1, MAXBYTES, file);
> + fclose(file);
> + }
> +
> + if (n == sizeof(uint32_t))
> + value = ((uint32_t *)buf)[0];
I would prefer *((uint32_t *)buf) but as I can see this is a convention
in this file, so, let's leave it as is.
> + else if (n == sizeof(uint64_t)) {
> + value = ((uint64_t *)buf)[0];
Ditto.
> + else {
> + fprintf(stderr, "%s node has invalid size: %d\n", fname, n);
> + ret = 0;
> + }
> +
> + if (pvalue)
> + *pvalue = value;
Add empty line here.
> + return ret;
> +}
> +
> /* Get devtree details and create exclude_range array
> * Also create usablemem_ranges for KEXEC_ON_CRASH
> */
> diff --git a/kexec/arch/ppc/kexec-ppc.h b/kexec/arch/ppc/kexec-ppc.h
> index 904cf48..0e52d18 100644
> --- a/kexec/arch/ppc/kexec-ppc.h
> +++ b/kexec/arch/ppc/kexec-ppc.h
> @@ -75,6 +75,7 @@ extern unsigned long dt_address_cells, dt_size_cells;
> extern int init_memory_region_info(void);
> extern int read_memory_region_limits(int fd, unsigned long long *start,
> unsigned long long *end);
> +extern int get_devtree_value (const char *fname, unsigned long long *pvalue);
Remove extra space between function name and parentheses.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 05/11] crashdump/m68k: Add get_crash_kernel_load_range() function
On Fri, Feb 10, 2017 at 04:07:36PM -0600, Eric DeVolder wrote: > From: Daniel Kiper > > Implement get_crash_kernel_load_range() in support of > print crash kernel region size option. Ditto. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 03/11] crashdump/cris: Add get_crash_kernel_load_range() function
On Fri, Feb 10, 2017 at 04:07:34PM -0600, Eric DeVolder wrote: > From: Daniel Kiper > > Implement get_crash_kernel_load_range() in support of > print crash kernel region size option. Ditto. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 02/11] crashdump/arm64: Add get_crash_kernel_load_range() function
Two nitpicks below...
On Fri, Feb 10, 2017 at 04:07:33PM -0600, Eric DeVolder wrote:
> Implement get_crash_kernel_load_range() in support of
> print crash kernel region size option.
This contradicts what is provided in patch body. You should rather say here
that you provide a stub to fulfill requirements of subsequent patch(es).
> Signed-off-by: Eric DeVolder
> ---
> kexec/arch/arm64/crashdump-arm64.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kexec/arch/arm64/crashdump-arm64.c
> b/kexec/arch/arm64/crashdump-arm64.c
> index d2272c8..b0e4713 100644
> --- a/kexec/arch/arm64/crashdump-arm64.c
> +++ b/kexec/arch/arm64/crashdump-arm64.c
> @@ -19,3 +19,9 @@ int is_crashkernel_mem_reserved(void)
> {
> return 0;
> }
> +
> +int get_crash_kernel_load_range(uint64_t *start, uint64_t *end)
> +{
> + /* Crash kernel region size is not exposed by the system */
Usually I add a full stop at the end of comment. Just my preference.
Otherwise:
Reviewed-by: Daniel Kiper
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 12/12] kexec: Add option to get crash kernel region size
On Mon, Feb 06, 2017 at 01:42:24PM -0600, Eric DeVolder wrote: > Here print_crashkernel_region_size() function is available on all archs (even > if the functionality is not implemented on some). So, we can safely use it in > arch independent code and export the functionality to the user space. > > Signed-off-by: Daniel Kiper > Signed-off-by: Eric DeVolder I think that patch #11 and #12 should be merged into one thing like it was in my original series. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 08/12] crashdump/ppc64: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:20PM -0600, Eric DeVolder wrote: > From: Daniel Kiper > > Follow similar x86 patch. > > Signed-off-by: Daniel Kiper > Signed-off-by: Eric DeVolder > > note ??? And please look at my comments for "crashdump/ppc: Add print_crashkernel_region_size() function". This patch requires similar fixes and cleanups like PPC one. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 07/12] crashdump/ppc: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:19PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Follow similar x86 patch.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> ---
> kexec/arch/ppc/crashdump-powerpc.c | 34 +++---
> kexec/arch/ppc/kexec-ppc.c | 23 +++
> kexec/arch/ppc/kexec-ppc.h | 1 +
> 3 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/kexec/arch/ppc/crashdump-powerpc.c
> b/kexec/arch/ppc/crashdump-powerpc.c
> index 3dc35eb..4c12452 100644
> --- a/kexec/arch/ppc/crashdump-powerpc.c
> +++ b/kexec/arch/ppc/crashdump-powerpc.c
> @@ -397,14 +397,34 @@ void add_usable_mem_rgns(unsigned long long base,
> unsigned long long size)
> usablemem_rgns.size, base, size);
> }
>
> +int get_crash_kernel_load_range(uint64_t *start, uint64_t *end)
> +{
> + unsigned long long value;
> + if
> (get_devtree_value("/proc/device-tree/chosen/linux,crashkernel-base",
> &value)) {
> + *start = value;
> + }
Curly brackets are not needed here.
> + else
> + *start = 0;
> + if
> (get_devtree_value("/proc/device-tree/chosen/linux,crashkernel-size",
> &value)) {
> + *end = *start + value - 1;
> + }
Ditto.
> + else
> + *end = 0;
I think that you should return -1 if get_devtree_value() returned error
here and there.
> + return 0;
> +}
> +
> int is_crashkernel_mem_reserved(void)
> {
> - int fd;
> -
> - fd = open("/proc/device-tree/chosen/linux,crashkernel-base", O_RDONLY);
> - if (fd < 0)
> - return 0;
> - close(fd);
> - return 1;
> + return
> get_devtree_value("/proc/device-tree/chosen/linux,crashkernel-base", NULL);
is_crashkernel_mem_reserved() change begs separate patch.
> }
>
> +void print_crashkernel_region_size(void)
> +{
> +uint64_t start = 0, end = 0;
> +
> +if (is_crashkernel_mem_reserved()) {
> +get_crash_kernel_load_range(&start, &end);
Please check value returned by get_crash_kernel_load_range() here.
> +printf("%llu\n", end - start + 1);
> +} else
> +printf("0\n");
> +}
> diff --git a/kexec/arch/ppc/kexec-ppc.c b/kexec/arch/ppc/kexec-ppc.c
> index d046110..78e8fb8 100644
> --- a/kexec/arch/ppc/kexec-ppc.c
> +++ b/kexec/arch/ppc/kexec-ppc.c
> @@ -423,6 +423,29 @@ err_out:
> return -1;
> }
>
> +int get_devtree_value (const char *fname, unsigned long long *pvalue)
Redundant space between function name and bracket.
> +{
> + /* Return 1 if fname/value valid, 0 otherwise */
> + FILE *file;
> + char buf[MAXBYTES];
> + int rcode = 1, n = -1;
s/rcode/ret/ And please follow convention and return -1 in case of error.
> + unsigned long long value = 0;
Lack of empty line here.
> + if ((file = fopen(fname, "r"))) {
> + n = fread(buf, 1, MAXBYTES, file);
> + fclose(file);
> + }
> + if (n == sizeof(uint32_t)) {
> + value = ((uint32_t *)buf)[0];
Curly brackets are not needed here.
> + } else if (n == sizeof(uint64_t)) {
> + value = ((uint64_t *)buf)[0];
Ditto.
> + } else {
> + fprintf(stderr, "%s node has invalid size: %d\n", fname, n);
> + rcode = 0;
> + }
> + if (pvalue) *pvalue = value;
"*pvalue = value;" should be in line below "if".
> + return rcode;
Please do not glue all lines into one giant block here and there. It is
unreadable.
> +}
> +
> /* Get devtree details and create exclude_range array
> * Also create usablemem_ranges for KEXEC_ON_CRASH
> */
> diff --git a/kexec/arch/ppc/kexec-ppc.h b/kexec/arch/ppc/kexec-ppc.h
> index 904cf48..69189f0 100644
> --- a/kexec/arch/ppc/kexec-ppc.h
> +++ b/kexec/arch/ppc/kexec-ppc.h
> @@ -71,6 +71,7 @@ extern unsigned char reuse_initrd;
> extern const char *ramdisk;
>
> /* Method to parse the memory/reg nodes in device-tree */
> +extern int get_devtree_value (const char *fname, unsigned long long *pvalue);
Put get_devtree_value() under read_memory_region_limits().
> extern unsigned long dt_address_cells, dt_size_cells;
> extern int init_memory_region_info(void);
> extern int read_memory_region_limits(int fd, unsigned long long *start,
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 05/12] crashdump/m68k: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:17PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Provide just print_crashkernel_region_size() stub. This way
> we can properly build kexec utility on m68k arch even
> if the functionality is not available on it.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> ---
> kexec/arch/m68k/kexec-m68k.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/kexec/arch/m68k/kexec-m68k.c b/kexec/arch/m68k/kexec-m68k.c
> index 372aa37..0cbf18e 100644
> --- a/kexec/arch/m68k/kexec-m68k.c
> +++ b/kexec/arch/m68k/kexec-m68k.c
> @@ -89,6 +89,11 @@ int is_crashkernel_mem_reserved(void)
> return 0;
> }
>
> +void print_crashkernel_region_size(void)
> +{
> + printf("Crashkernel functionality is not available.\n");
Ditto.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 03/12] crashdump/cris: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:15PM -0600, Eric DeVolder wrote:
> From: Daniel Kiper
>
> Provide just print_crashkernel_region_size() stub. This way
> we can properly build kexec utility on cris arch even
> if the functionality is not available on it.
>
> Signed-off-by: Daniel Kiper
> Signed-off-by: Eric DeVolder
> ---
> kexec/arch/cris/kexec-cris.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/kexec/arch/cris/kexec-cris.c b/kexec/arch/cris/kexec-cris.c
> index 4ac2f89..5601d8e 100644
> --- a/kexec/arch/cris/kexec-cris.c
> +++ b/kexec/arch/cris/kexec-cris.c
> @@ -77,6 +77,11 @@ int is_crashkernel_mem_reserved(void)
> return 0;
> }
>
> +void print_crashkernel_region_size(void)
> +{
> + printf("Crashkernel functionality is not available.\n");
Error message is a bit confusing. What about "Crash kernel region size cannot
be printed because this info is not exposed by the system."?
Or simpler: Crash kernel region size is not exposed by the system.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 01/12] crashdump/x86: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:13PM -0600, Eric DeVolder wrote: > From: Daniel Kiper > > Crash kernel region size is available via sysfs on Linux running on > bare metal. However, this does not work when Linux runs as Xen dom0. > In this case Xen crash kernel region size should be established using > __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does > not make a lot of sense in Xen dom0). Sadly hypercalls are not easily > accessible using shell scripts or something like that. Potentially we > can check "xl dmesg" output for crashkernel option but this is not nice. > So, let's add this functionality, for Linux running on bare metal and > as Xen dom0, to kexec-tools. This way kdump scripts may establish crash > kernel region size in one way regardless of platform. All burden of > platform detection lies on kexec-tools. > > Figure (and unit) displayed by this new kexec-tools functionality is > the same as one taken from /sys/kernel/kexec_crash_size. > > This patch just adds print_crashkernel_region_size() function, which > prints crash kernel region size, for x86 arch. Next patches will add > same named function for other archs supported by kexec-tools. Last patch > of this series will export this functionality to the userspace via > separate kexec utility option. > > Signed-off-by: Daniel Kiper > Signed-off-by: Eric DeVolder > --- > v0: Interal version. > v1: Posted to kexec-tools mailing list > v2: Incorporated feedback: > - utilize the is_crashkernel_mem_reserved() function common in all archs > - for ppc and ppc64, utilize device-tree values to print size > - for unsupported architectures, print appropriate message Please do not blindly copy description of changes from patch #00. I think it makes more sense if you tell us what really has been changed in this patch. Otherwise LGTM. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 00/12] crashdump: Add print_crashkernel_region_size() function
On Mon, Feb 06, 2017 at 01:42:12PM -0600, Eric DeVolder wrote: > Crash kernel region size is available via sysfs on Linux running on > bare metal. However, this does not work when Linux runs as Xen dom0. > In this case Xen crash kernel region size should be established using > __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does > not make a lot of sense in Xen dom0). Sadly hypercalls are not easily > accessible using shell scripts or something like that. Potentially we > can check "xl dmesg" output for crashkernel option but this is not nice. > So, let's add this functionality, for Linux running on bare metal and > as Xen dom0, to kexec-tools. This way kdump scripts may establish crash > kernel region size in one way regardless of platform. All burden of > platform detection lies on kexec-tools. > > Figure (and unit) displayed by this new kexec-tools functionality is > the same as one taken from /sys/kernel/kexec_crash_size. > > This patch just adds print_crashkernel_region_size() function, which > prints crash kernel region size, for x86 arch. Next patches will add > same named function for other archs supported by kexec-tools. Last patch > of this series will export this functionality to the userspace via > separate kexec utility option. > > Signed-off-by: Daniel Kiper > Signed-off-by: Eric DeVolder This does not need to have SOB. > --- > v0: Interal version. > v1: Posted to kexec-tools mailing list > v2: Incorporated feedback: > - utilize the is_crashkernel_mem_reserved() function common in all archs > - for ppc and ppc64, utilize device-tree values to print size > - for unsupported architectures, print appropriate message Next time please provide a list of patches (numbers are OK) changed in comparison to earlier version. Results of "git diff --stat master.." and "git shortlog master.." (I assume that you based patches on master) are also nice to have at the bottom of patch #00. Daniel ___ 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
On Thu, Jan 26, 2017 at 11:22:19AM +0100, Simon Horman wrote:
> On Thu, Jan 26, 2017 at 12:02:48AM +0100, Daniel Kiper wrote:
> > 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, &p, 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, &p, 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.
>
> It did not seem to help when I tried.
>
> I'm happy with things the way they are and I will apply this patch.
Great! Thanks.
Daniel
___
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
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, &p, 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, &p, 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 v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
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
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
Re: [PATCH v3] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
On Tue, Jan 24, 2017 at 12:55:35PM -0600, Eric DeVolder wrote:
> 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
> ---
> Note: The corresponding Xen changes have been committed
> to the Xen staging branch. Follow this thread:
> https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html
>
> CC: Andrew Cooper
> 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
> ---
> configure.ac | 8 ++-
> kexec/kexec-xen.c | 26 +++
> kexec/kexec.8 | 6 ++
> kexec/kexec.c | 62
> ---
> kexec/kexec.h | 5 -
> 5 files changed, 98 insertions(+), 9 deletions(-)
>
> 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.
> 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.
> +.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/ke
Re: [PATCH v2] kexec: implemented XEN KEXEC STATUS to determine if an image is loaded
On Sat, Jan 21, 2017 at 09:43:19AM +0800, Baoquan He wrote: > Hi, > > I don't strongly oppose against this, but could you tell what you have > met makes the kexec_loaded/kexec_crash_loaded checking not convenient to > you? 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. I hope that helps. > On 01/20/17 at 11:03am, Eric DeVolder wrote: > > Instead of the scripts having to poke at various fields we can > > provide that functionality via the -S parameter. > > > > 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 > > --- > > Note: The corresponding Xen changes have been committed > > to the Xen staging branch. Follow this thread: > > https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg01570.html > > > > CC: Andrew Cooper > > 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 > > --- > > configure.ac | 8 ++- > > kexec/kexec-xen.c | 26 +++ > > kexec/kexec.8 | 6 ++ > > kexec/kexec.c | 62 > > --- > > kexec/kexec.h | 5 - > > 5 files changed, 98 insertions(+), 9 deletions(-) > > > > 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 Eric, I have a feeling that you should add en extra indentation for lines starting from "+if test "$have_xenctrl_h"..." and ending at "+fi". Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 1/2] xen/kexec: Find out whether an kexec type is loaded.
On Mon, Nov 14, 2016 at 05:12:52PM -0500, Konrad Rzeszutek Wilk 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.
>
> Signed-off-by: Konrad Rzeszutek Wilk
> ---
> v0: Internal version.
> v1: Dropped Reviewed-by, posting on xen-devel.
>
> CC: Elena Ufimtseva
> CC: Daniel Kiper
> ---
> tools/libxc/include/xenctrl.h | 8
> tools/libxc/xc_kexec.c| 27 +++
> xen/common/kexec.c| 25 +
> xen/include/public/kexec.h| 11 +++
> 4 files changed, 71 insertions(+)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 2c83544..aa5d798 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2574,6 +2574,14 @@ 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 can be either KEXEC_TYPE_DEFAULT or KEXEC_TYPE_CRASH.
> + * If zero is returned that means the image is loaded for the type.
> + */
> +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 1cceb5d..95d36ff 100644
> --- a/tools/libxc/xc_kexec.c
> +++ b/tools/libxc/xc_kexec.c
> @@ -126,3 +126,30 @@ out:
>
> return ret;
> }
> +
> +int xc_kexec_status(xc_interface *xch, int type)
> +{
> +DECLARE_HYPERCALL;
> +DECLARE_HYPERCALL_BUFFER(xen_kexec_status_t, status);
> +int ret = -1;
> +
> +status = xc_hypercall_buffer_alloc(xch, status, sizeof(*status));
> +if ( status == NULL )
> +{
> +PERROR("Count not alloc buffer for kexec status hypercall");
Could not? It looks that you copied this from existing code. Could you
send separate patch fixing this issue in two other places too?
> +goto out;
> +}
> +
> +status->type = type;
> +
> +hypercall.op = __HYPERVISOR_kexec_op;
> +hypercall.arg[0] = KEXEC_CMD_kexec_status;
> +hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(status);
> +
> +ret = do_xen_hypercall(xch, &hypercall);
> +
> +out:
> +xc_hypercall_buffer_free(xch, status);
> +
> +return ret;
> +}
> diff --git a/xen/common/kexec.c b/xen/common/kexec.c
> index c83d48f..1148f85 100644
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -1169,6 +1169,28 @@ 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, pos;
> +
> +if ( unlikely(copy_from_guest(&status, uarg, 1)) )
> +return -EFAULT;
> +
> +if ( test_bit(KEXEC_FLAG_IN_PROGRESS, &kexec_flags) )
> +return -EBUSY;
> +
> +if ( kexec_load_get_bits(status.type, &base, &bit) )
> +return -EINVAL;
> +
> +pos = (test_bit(bit, &kexec_flags) != 0);
> +
> +if ( !test_bit(base + pos, &kexec_flags) )
> +return -ENOENT;
> +
> +return 0;
> +}
> +
> static int do_kexec_op_internal(unsigned long op,
> XEN_GUEST_HANDLE_PARAM(void) uarg,
> bool_t compat)
> @@ -1208,6 +1230,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..29dcb5d 100644
> --- a/xen/include/public/kexec.h
> +++ b/xen/include/public/kexec.h
> @@ -227,6 +227,17 @@ 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. An return value of
> + * zero indicates success while XEN_ENODEV implies no image loaded.
It seems to me that you thought about -ENOENT instead of XEN_ENODEV.
If you fix both things then Reviewed-by: Daniel Kiper
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [Xen-devel] [PATCH] xen: Add support for dom0 with Linux kernel 3.19 and newer
On Fri, Jan 22, 2016 at 10:03:34AM +, David Vrabel wrote: > On 21/01/16 20:13, Daniel Kiper wrote: > > Linux kernel commit 054954eb051f35e74b75a566a96fe756015352c8 > > (xen: switch to linear virtual mapped sparse p2m list), which > > appeared in 3.19, introduced linear virtual mapped sparse p2m > > list. If readmem() reads p2m then it access this list using > > physical addresses. Sadly, VMA to physical address translation > > in crash requires access to p2m list. This means that we have > > a chicken and egg problem. In general this issue must be solved > > by introducing some changes in libxl, Linux kernel and crash > > (I have added this task to my long TODO list). However, in dom0 > > case we can use crash_xen_info_t.dom0_pfn_to_mfn_frame_list_list > > which is available out of the box. So, let's use it and make > > at least some users happy. > > I'm confused. How does a virtual address to (pseudo-)physical address > lookup require access to the p2m? Surely this is a walk of the page > tables followed by a M2P lookup on the MFN in the L1 PTE? Correct but crash does it a bit backward and scans p2m. I am not sure way, however, I am going to look at it during work on PV guests support. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] xen: Add support for dom0 with Linux kernel 3.19 and newer
Linux kernel commit 054954eb051f35e74b75a566a96fe756015352c8
(xen: switch to linear virtual mapped sparse p2m list), which
appeared in 3.19, introduced linear virtual mapped sparse p2m
list. If readmem() reads p2m then it access this list using
physical addresses. Sadly, VMA to physical address translation
in crash requires access to p2m list. This means that we have
a chicken and egg problem. In general this issue must be solved
by introducing some changes in libxl, Linux kernel and crash
(I have added this task to my long TODO list). However, in dom0
case we can use crash_xen_info_t.dom0_pfn_to_mfn_frame_list_list
which is available out of the box. So, let's use it and make
at least some users happy.
Signed-off-by: Daniel Kiper
---
kernel.c | 81 ++--
xen_dom0.c |3 ++-
xen_dom0.h |2 ++
3 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/kernel.c b/kernel.c
index 5ce2fb9..b07149e 100644
--- a/kernel.c
+++ b/kernel.c
@@ -17,6 +17,7 @@
#include "defs.h"
#include "xen_hyper_defs.h"
+#include "xen_dom0.h"
#include
#include
#include
@@ -61,6 +62,7 @@ static int restore_stack(struct bt_info *);
static ulong __xen_m2p(ulonglong, ulong);
static ulong __xen_pvops_m2p_l2(ulonglong, ulong);
static ulong __xen_pvops_m2p_l3(ulonglong, ulong);
+static ulong __xen_pvops_m2p_hyper(ulonglong, ulong);
static int search_mapping_page(ulong, ulong *, ulong *, ulong *);
static void read_in_kernel_config_err(int, char *);
static void BUG_bytes_init(void);
@@ -175,6 +177,9 @@ kernel_init()
&kt->pvops_xen.p2m_mid_missing);
get_symbol_data("p2m_missing", sizeof(ulong),
&kt->pvops_xen.p2m_missing);
+ } else if (symbol_exists("xen_p2m_addr")) {
+ if (!XEN_CORE_DUMPFILE())
+ error(FATAL, "p2m array in new format is
unreadable.");
} else {
kt->pvops_xen.p2m_top_entries =
get_array_length("p2m_top", NULL, 0);
kt->pvops_xen.p2m_top = symbol_value("p2m_top");
@@ -5850,12 +5855,14 @@ no_cpu_flags:
else
fprintf(fp, "\n");
- fprintf(fp, " pvops_xen:\n");
- fprintf(fp, "p2m_top: %lx\n",
kt->pvops_xen.p2m_top);
- fprintf(fp, "p2m_top_entries: %d\n",
kt->pvops_xen.p2m_top_entries);
- if (symbol_exists("p2m_mid_missing"))
- fprintf(fp, "p2m_mid_missing: %lx\n",
kt->pvops_xen.p2m_mid_missing);
- fprintf(fp, "p2m_missing: %lx\n",
kt->pvops_xen.p2m_missing);
+ if (!symbol_exists("xen_p2m_addr")) {
+ fprintf(fp, " pvops_xen:\n");
+ fprintf(fp, "p2m_top: %lx\n",
kt->pvops_xen.p2m_top);
+ fprintf(fp, "p2m_top_entries: %d\n",
kt->pvops_xen.p2m_top_entries);
+ if (symbol_exists("p2m_mid_missing"))
+ fprintf(fp, "p2m_mid_missing: %lx\n",
kt->pvops_xen.p2m_mid_missing);
+ fprintf(fp, "p2m_missing: %lx\n",
kt->pvops_xen.p2m_missing);
+ }
}
/*
@@ -8873,6 +8880,12 @@ __xen_m2p(ulonglong machine, ulong mfn)
ulong c, i, kmfn, mapping, p, pfn;
ulong start, end;
ulong *mp = (ulong *)kt->m2p_page;
+ int memtype;
+
+ if (XEN_CORE_DUMPFILE() && symbol_exists("xen_p2m_addr"))
+ memtype = PHYSADDR;
+ else
+ memtype = KVADDR;
/*
* Check the FIFO cache first.
@@ -8883,13 +8896,19 @@ __xen_m2p(ulonglong machine, ulong mfn)
(mfn <= kt->p2m_mapping_cache[c].end))) {
if (kt->p2m_mapping_cache[c].mapping !=
kt->last_mapping_read) {
- if (!readmem(kt->p2m_mapping_cache[c].mapping,
KVADDR,
+ if (memtype == PHYSADDR)
+ pc->curcmd_flags |= XEN_MACHINE_ADDR;
+
+ if (!readmem(kt->p2m_mapping_cache[c].mapping,
memtype,
mp, PAGESIZE(), "phys_to_machine_mapping
page (cached)",
RETURN_ON_ERROR))
error(FATAL, "cannot access "
"phys_to_machine_mapping page\n");
else
kt->las
[PATCH 13/14] crashdump/sh: Add print_crashkernel_region_size() function
Follow similar x86 patch.
Signed-off-by: Daniel Kiper
---
kexec/arch/sh/crashdump-sh.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/kexec/arch/sh/crashdump-sh.c b/kexec/arch/sh/crashdump-sh.c
index 9e6af6b..6556eb1 100644
--- a/kexec/arch/sh/crashdump-sh.c
+++ b/kexec/arch/sh/crashdump-sh.c
@@ -178,3 +178,13 @@ int is_crashkernel_mem_reserved(void)
return parse_iomem_single("Crash kernel\n", &start, &end) == 0 ?
(start != end) : 0;
}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (!parse_iomem_single("Crash kernel\n", &start, &end) && start != end)
+ printf("%lu\n", end - start + 1);
+ else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 11/14] crashdump/ppc64: Add print_crashkernel_region_size() function
Provide just print_crashkernel_region_size() stub. This way
we can properly build kexec utility on ppc64 arch even
if the functionality is not available on it.
Signed-off-by: Daniel Kiper
---
kexec/arch/ppc64/crashdump-ppc64.c |5 +
1 file changed, 5 insertions(+)
diff --git a/kexec/arch/ppc64/crashdump-ppc64.c
b/kexec/arch/ppc64/crashdump-ppc64.c
index b3c8928..49af7a6 100644
--- a/kexec/arch/ppc64/crashdump-ppc64.c
+++ b/kexec/arch/ppc64/crashdump-ppc64.c
@@ -527,6 +527,11 @@ int is_crashkernel_mem_reserved(void)
return 1;
}
+void print_crashkernel_region_size(void)
+{
+ printf("-1\n");
+}
+
#if 0
static int sort_regions(mem_rgns_t *rgn)
{
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 03/14] crashdump: Remove stray get_crashkernel_region() declaration
Signed-off-by: Daniel Kiper --- kexec/crashdump.h |1 - 1 file changed, 1 deletion(-) diff --git a/kexec/crashdump.h b/kexec/crashdump.h index 95f1f0c..320767b 100644 --- a/kexec/crashdump.h +++ b/kexec/crashdump.h @@ -1,7 +1,6 @@ #ifndef CRASHDUMP_H #define CRASHDUMP_H -int get_crashkernel_region(uint64_t *start, uint64_t *end); extern int get_crash_notes_per_cpu(int cpu, uint64_t *addr, uint64_t *len); extern int get_kernel_vmcoreinfo(uint64_t *addr, uint64_t *len); extern int get_xen_vmcoreinfo(uint64_t *addr, uint64_t *len); -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 08/14] crashdump/m68k: Add print_crashkernel_region_size() function
Provide just print_crashkernel_region_size() stub. This way
we can properly build kexec utility on m68k arch even
if the functionality is not available on it.
Signed-off-by: Daniel Kiper
---
kexec/arch/m68k/kexec-m68k.c |5 +
1 file changed, 5 insertions(+)
diff --git a/kexec/arch/m68k/kexec-m68k.c b/kexec/arch/m68k/kexec-m68k.c
index 372aa37..a9915a0 100644
--- a/kexec/arch/m68k/kexec-m68k.c
+++ b/kexec/arch/m68k/kexec-m68k.c
@@ -89,6 +89,11 @@ int is_crashkernel_mem_reserved(void)
return 0;
}
+void print_crashkernel_region_size(void)
+{
+ printf("-1\n");
+}
+
unsigned long virt_to_phys(unsigned long addr)
{
return addr + m68k_memoffset;
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 05/14] crashdump/arm: Add print_crashkernel_region_size() function
Follow similar x86 patch.
Signed-off-by: Daniel Kiper
---
kexec/arch/arm/crashdump-arm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/kexec/arch/arm/crashdump-arm.c b/kexec/arch/arm/crashdump-arm.c
index b523e5f..b3e93ad 100644
--- a/kexec/arch/arm/crashdump-arm.c
+++ b/kexec/arch/arm/crashdump-arm.c
@@ -414,3 +414,13 @@ int is_crashkernel_mem_reserved(void)
return 0;
}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (!parse_iomem_single("Crash kernel\n", &start, &end) && start != end)
+ printf("%lu\n", end - start + 1);
+ else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 14/14] kexec: Add option to get crash kernel region size
Here print_crashkernel_region_size() function is available on all archs (even
if the functionality is not implemented on some). So, we can safely use it in
arch independent code and export the functionality to the user space.
Signed-off-by: Daniel Kiper
---
kexec/kexec.8 |3 +++
kexec/kexec.c |4
kexec/kexec.h |5 -
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..e65345e 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -173,6 +173,9 @@ Load a helper image to jump back to original kernel.
.TP
.BI \-\-reuseinitrd
Reuse initrd from first boot.
+.TP
+.BI \-\-print-ckr-size
+Print crash kernel region size. -1 means that this functionality is not
implemented.
.SH SUPPORTED KERNEL FILE TYPES AND OPTIONS
diff --git a/kexec/kexec.c b/kexec/kexec.c
index 20dd93d..a27f596 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -960,6 +960,7 @@ void usage(void)
" --mem-max= Specify the highest memory address to\n"
" load code into.\n"
" --reuseinitrdReuse initrd from first boot.\n"
+ " --print-ckr-size Print crash kernel region size.\n"
" --load-preserve-context Load the new kernel and preserve\n"
" context of current kernel during kexec.\n"
" --load-jump-back-helper Load a helper image to jump back\n"
@@ -1345,6 +1346,9 @@ int main(int argc, char *argv[])
case OPT_KEXEC_FILE_SYSCALL:
/* We already parsed it. Nothing to do. */
break;
+ case OPT_PRINT_CKR_SIZE:
+ print_crashkernel_region_size();
+ return 0;
default:
break;
}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index c02ac8f..42a602d 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -224,7 +224,8 @@ extern int file_types;
#define OPT_LOAD_PRESERVE_CONTEXT 259
#define OPT_LOAD_JUMP_BACK_HELPER 260
#define OPT_ENTRY 261
-#define OPT_MAX262
+#define OPT_PRINT_CKR_SIZE 262
+#define OPT_MAX263
#define KEXEC_OPTIONS \
{ "help", 0, 0, OPT_HELP }, \
{ "version",0, 0, OPT_VERSION }, \
@@ -244,6 +245,7 @@ extern int file_types;
{ "reuseinitrd",0, 0, OPT_REUSE_INITRD }, \
{ "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \
{ "debug", 0, 0, OPT_DEBUG }, \
+ { "print-ckr-size", 0, 0, OPT_PRINT_CKR_SIZE }, \
#define KEXEC_OPT_STR "h?vdfxyluet:ps"
@@ -290,6 +292,7 @@ int arch_compat_trampoline(struct kexec_info *info);
void arch_update_purgatory(struct kexec_info *info);
int is_crashkernel_mem_reserved(void);
int get_crash_kernel_load_range(uint64_t *start, uint64_t *end);
+void print_crashkernel_region_size(void);
char *get_command_line(void);
int kexec_iomem_for_each_line(char *match,
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 06/14] crashdump/cris: Add print_crashkernel_region_size() function
Provide just print_crashkernel_region_size() stub. This way
we can properly build kexec utility on cris arch even
if the functionality is not available on it.
Signed-off-by: Daniel Kiper
---
kexec/arch/cris/kexec-cris.c |5 +
1 file changed, 5 insertions(+)
diff --git a/kexec/arch/cris/kexec-cris.c b/kexec/arch/cris/kexec-cris.c
index 4ac2f89..8c62191 100644
--- a/kexec/arch/cris/kexec-cris.c
+++ b/kexec/arch/cris/kexec-cris.c
@@ -77,6 +77,11 @@ int is_crashkernel_mem_reserved(void)
return 0;
}
+void print_crashkernel_region_size(void)
+{
+ printf("-1\n");
+}
+
unsigned long virt_to_phys(unsigned long addr)
{
return (addr) & 0x7fff;
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 10/14] crashdump/ppc: Add print_crashkernel_region_size() function
Provide just print_crashkernel_region_size() stub. This way
we can properly build kexec utility on ppc arch even
if the functionality is not available on it.
Signed-off-by: Daniel Kiper
---
kexec/arch/ppc/crashdump-powerpc.c |4
1 file changed, 4 insertions(+)
diff --git a/kexec/arch/ppc/crashdump-powerpc.c
b/kexec/arch/ppc/crashdump-powerpc.c
index 3dc35eb..19c4a61 100644
--- a/kexec/arch/ppc/crashdump-powerpc.c
+++ b/kexec/arch/ppc/crashdump-powerpc.c
@@ -408,3 +408,7 @@ int is_crashkernel_mem_reserved(void)
return 1;
}
+void print_crashkernel_region_size(void)
+{
+ printf("-1\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 07/14] crashdump/ia64: Add print_crashkernel_region_size() function
Follow similar x86 patch.
Signed-off-by: Daniel Kiper
---
kexec/arch/ia64/crashdump-ia64.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/kexec/arch/ia64/crashdump-ia64.c b/kexec/arch/ia64/crashdump-ia64.c
index 726c9f4..07de42a 100644
--- a/kexec/arch/ia64/crashdump-ia64.c
+++ b/kexec/arch/ia64/crashdump-ia64.c
@@ -286,3 +286,13 @@ int is_crashkernel_mem_reserved(void)
return parse_iomem_single("Crash kernel\n", &start,
&end) == 0 ? (start != end) : 0;
}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (!parse_iomem_single("Crash kernel\n", &start, &end) && start != end)
+ printf("%lu\n", end - start + 1);
+ else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 09/14] crashdump/mips: Add print_crashkernel_region_size() function
Follow similar x86 patch.
Signed-off-by: Daniel Kiper
---
kexec/arch/mips/crashdump-mips.c |9 +
1 file changed, 9 insertions(+)
diff --git a/kexec/arch/mips/crashdump-mips.c b/kexec/arch/mips/crashdump-mips.c
index dc68cb4..450fa13 100644
--- a/kexec/arch/mips/crashdump-mips.c
+++ b/kexec/arch/mips/crashdump-mips.c
@@ -377,3 +377,12 @@ int is_crashkernel_mem_reserved(void)
(start != end) : 0;
}
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (!parse_iomem_single("Crash kernel\n", &start, &end) && start != end)
+ printf("%lu\n", end - start + 1);
+ else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 04/14] crashdump/x86: Add print_crashkernel_region_size() function
Crash kernel region size is available via sysfs on Linux running on
bare metal. However, this does not work when Linux runs as Xen dom0.
In this case Xen crash kernel region size should be established using
__HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does
not make a lot of sense in Xen dom0). Sadly hypercalls are not easily
accessible using shell scripts or something like that. Potentially we
can check "xl dmesg" output for crashkernel option but this is not nice.
So, let's add this functionality, for Linux running on bare metal and
as Xen dom0, to kexec-tools. This way kdump scripts may establish crash
kernel region size in one way regardless of platform. All burden of
platform detection lies on kexec-tools.
Figure (and unit) displayed by this new kexec-tools functionality is
the same as one taken from /sys/kernel/kexec_crash_size.
This patch just adds print_crashkernel_region_size() function, which
prints crash kernel region size, for x86 arch. Next patches will add
same named function for other archs supported by kexec-tools. Last patch
of this series will export this functionality to the userspace via
separate kexec utility option.
Signed-off-by: Daniel Kiper
---
kexec/arch/i386/crashdump-x86.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index bbc0f35..10a56a8 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -1056,7 +1056,7 @@ static int crashkernel_mem_callback(void *UNUSED(data),
int nr,
return 0;
}
-int is_crashkernel_mem_reserved(void)
+static int get_crashkernel_region(void)
{
int ret;
@@ -1079,3 +1079,19 @@ int is_crashkernel_mem_reserved(void)
return !!crash_reserved_mem_nr;
}
+
+int is_crashkernel_mem_reserved(void)
+{
+ return get_crashkernel_region();
+}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (get_crashkernel_region()) {
+ get_crash_kernel_load_range(&start, &end);
+ printf("%lu\n", end - start + 1);
+ } else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 12/14] crashdump/s390: Add print_crashkernel_region_size() function
Follow similar x86 patch.
Signed-off-by: Daniel Kiper
---
kexec/arch/s390/kexec-s390.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/kexec/arch/s390/kexec-s390.c b/kexec/arch/s390/kexec-s390.c
index 074575e..212a64f 100644
--- a/kexec/arch/s390/kexec-s390.c
+++ b/kexec/arch/s390/kexec-s390.c
@@ -262,3 +262,13 @@ int is_crashkernel_mem_reserved(void)
return parse_iomem_single("Crash kernel\n", &start, &end) == 0 ?
(start != end) : 0;
}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t start, end;
+
+ if (!parse_iomem_single("Crash kernel\n", &start, &end) && start != end)
+ printf("%lu\n", end - start + 1);
+ else
+ printf("0\n");
+}
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 02/14] kexec: Remove redundant space from help message
Signed-off-by: Daniel Kiper
---
kexec/kexec.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kexec/kexec.c b/kexec/kexec.c
index f0bd527..20dd93d 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -969,7 +969,7 @@ void usage(void)
" preserve context)\n"
" 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"
+ " -d, --debug Enable debugging to help spot a
failure.\n"
"\n"
"Supported kernel file types and options: \n");
for (i = 0; i < file_types; i++) {
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 00/14] kexec: Add option to get crash kernel region size
Crash kernel region size is available via sysfs on Linux running on bare metal. However, this does not work when Linux runs as Xen dom0. In this case Xen crash kernel region size should be established using __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does not make a lot of sense in Xen dom0). Sadly hypercalls are not easily accessible using shell scripts or something like that. Potentially we can check "xl dmesg" output for crashkernel option but this is not nice. So, let's add this functionality, for Linux running on bare metal and as Xen dom0, to kexec-tools. This way kdump scripts may establish crash kernel region size in one way regardless of platform. All burden of platform detection lies on kexec-tools. Figure (and unit) displayed by this new kexec-tools functionality is the same as one taken from /sys/kernel/kexec_crash_size. First three patches are fixes and cleanups. The rest provides above described functionality for all supported platforms. Daniel kexec/arch/arm/crashdump-arm.c | 10 ++ kexec/arch/cris/kexec-cris.c |5 + kexec/arch/i386/crashdump-x86.c| 18 +- kexec/arch/ia64/crashdump-ia64.c | 10 ++ kexec/arch/m68k/kexec-m68k.c |5 + kexec/arch/mips/crashdump-mips.c |9 + kexec/arch/ppc/crashdump-powerpc.c |4 kexec/arch/ppc64/crashdump-ppc64.c |5 + kexec/arch/s390/kexec-s390.c | 10 ++ kexec/arch/sh/crashdump-sh.c | 10 ++ kexec/crashdump.h |1 - kexec/kexec.8 |3 +++ kexec/kexec.c |6 +- kexec/kexec.h |5 - purgatory/Makefile |2 +- 15 files changed, 98 insertions(+), 5 deletions(-) Daniel Kiper (14): purgatory: Add purgatory.map and purgatory.ro.sym to clean recipe kexec: Remove redundant space from help message crashdump: Remove stray get_crashkernel_region() declaration crashdump/x86: Add print_crashkernel_region_size() function crashdump/arm: Add print_crashkernel_region_size() function crashdump/cris: Add print_crashkernel_region_size() function crashdump/ia64: Add print_crashkernel_region_size() function crashdump/m68k: Add print_crashkernel_region_size() function crashdump/mips: Add print_crashkernel_region_size() function crashdump/ppc: Add print_crashkernel_region_size() function crashdump/ppc64: Add print_crashkernel_region_size() function crashdump/s390: Add print_crashkernel_region_size() function crashdump/sh: Add print_crashkernel_region_size() function kexec: Add option to get crash kernel region size ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 01/14] purgatory: Add purgatory.map and purgatory.ro.sym to clean recipe
Signed-off-by: Daniel Kiper --- purgatory/Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/purgatory/Makefile b/purgatory/Makefile index 2b5c061..caea7ea 100644 --- a/purgatory/Makefile +++ b/purgatory/Makefile @@ -33,7 +33,7 @@ PURGATORY_SRCS+=$($(ARCH)_PURGATORY_SRCS) PURGATORY_OBJS = $(call objify, $(PURGATORY_SRCS)) purgatory/sha256.o PURGATORY_DEPS = $(call depify, $(PURGATORY_OBJS)) -clean += $(PURGATORY_OBJS) $(PURGATORY_DEPS) $(PURGATORY) +clean += $(PURGATORY_OBJS) $(PURGATORY_DEPS) $(PURGATORY) $(PURGATORY_MAP) $(PURGATORY).sym -include $(PURGATORY_DEPS) -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] x86/xen: Build memory type conversion code conditionally
On Mon, Nov 30, 2015 at 08:56:14AM +0900, Simon Horman wrote: > On Sat, Nov 28, 2015 at 09:17:18PM +0100, Daniel Kiper wrote: > > Xen is not aware of E820_PMEM and E820_PRAM memory types, so, build > > simply fails. Hence, build code referencing them conditionally. > > > > Signed-off-by: Daniel Kiper > > I believe this is fixed by > "x86: Make sure E820_PM[AE]M are defined if needed" > which I applied a few moments ago. Yep, it works. However, I think that separate #ifdef should be for E820_PMEM and E820_PRAM. Just my $0.05. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH] crashdump/x86: Add option to get crash kernel region size
On Mon, Nov 30, 2015 at 08:55:32AM +0900, Simon Horman wrote: > On Sat, Nov 28, 2015 at 09:14:04PM +0100, Daniel Kiper wrote: > > Crash kernel region size is available via sysfs on Linux running on > > bare metal. However, this does not work when Linux runs as Xen dom0. > > In this case Xen crash kernel region size should be established using > > __HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does > > not make a lot of sense in Xen dom0). Sadly hypercalls are not easily > > accessible using shell scripts or something like that. Potentially we > > can check "xl dmesg" output for crashkernel option but this is not nice. > > So, let's add this functionality, for Linux running on bare metal and > > as Xen dom0, to kexec-tools. This way kdump scripts may establish crash > > kernel region size in one way regardless of platform. All burden of > > platform detection lies on kexec-tools. > > > > Figure (and unit) displayed by this new kexec-tools functionality is > > the same as one taken from /sys/kernel/kexec_crash_size. > > > > This functionality is available on x86 platform only. If idea is acceptable > > then I can prepare patches for other platforms (if it is possible and make > > sense) and repost them as fully flagged patch series. > > > > Signed-off-by: Daniel Kiper > > Thanks, applied. Thanks a lot. However, this patch itself breaks build on other platforms. That is why I posted it as a RFC. To fix this I can post similar patches for other platforms (this will happen probably tomorrow or on Wednesday but no later than by the end of this week). Or you can revert this patch and than I repost it as a part of fully functional patch series. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[RFC PATCH] crashdump/x86: Add option to get crash kernel region size
Crash kernel region size is available via sysfs on Linux running on
bare metal. However, this does not work when Linux runs as Xen dom0.
In this case Xen crash kernel region size should be established using
__HYPERVISOR_kexec_op hypercall (Linux kernel kexec functionality does
not make a lot of sense in Xen dom0). Sadly hypercalls are not easily
accessible using shell scripts or something like that. Potentially we
can check "xl dmesg" output for crashkernel option but this is not nice.
So, let's add this functionality, for Linux running on bare metal and
as Xen dom0, to kexec-tools. This way kdump scripts may establish crash
kernel region size in one way regardless of platform. All burden of
platform detection lies on kexec-tools.
Figure (and unit) displayed by this new kexec-tools functionality is
the same as one taken from /sys/kernel/kexec_crash_size.
This functionality is available on x86 platform only. If idea is acceptable
then I can prepare patches for other platforms (if it is possible and make
sense) and repost them as fully flagged patch series.
Signed-off-by: Daniel Kiper
---
kexec/arch/i386/crashdump-x86.c | 18 +-
kexec/crashdump.h |1 -
kexec/kexec.8 |3 +++
kexec/kexec.c |4
kexec/kexec.h |5 -
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index bbc0f35..4b31112 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -1056,7 +1056,7 @@ static int crashkernel_mem_callback(void *UNUSED(data),
int nr,
return 0;
}
-int is_crashkernel_mem_reserved(void)
+static int get_crashkernel_region(void)
{
int ret;
@@ -1079,3 +1079,19 @@ int is_crashkernel_mem_reserved(void)
return !!crash_reserved_mem_nr;
}
+
+int is_crashkernel_mem_reserved(void)
+{
+ return get_crashkernel_region();
+}
+
+void print_crashkernel_region_size(void)
+{
+ uint64_t end, start;
+
+ if (get_crashkernel_region()) {
+ get_crash_kernel_load_range(&start, &end);
+ printf("%lu\n", end - start + 1);
+ } else
+ printf("0\n");
+}
diff --git a/kexec/crashdump.h b/kexec/crashdump.h
index 95f1f0c..320767b 100644
--- a/kexec/crashdump.h
+++ b/kexec/crashdump.h
@@ -1,7 +1,6 @@
#ifndef CRASHDUMP_H
#define CRASHDUMP_H
-int get_crashkernel_region(uint64_t *start, uint64_t *end);
extern int get_crash_notes_per_cpu(int cpu, uint64_t *addr, uint64_t *len);
extern int get_kernel_vmcoreinfo(uint64_t *addr, uint64_t *len);
extern int get_xen_vmcoreinfo(uint64_t *addr, uint64_t *len);
diff --git a/kexec/kexec.8 b/kexec/kexec.8
index 4d0c1d1..ba7d096 100644
--- a/kexec/kexec.8
+++ b/kexec/kexec.8
@@ -173,6 +173,9 @@ Load a helper image to jump back to original kernel.
.TP
.BI \-\-reuseinitrd
Reuse initrd from first boot.
+.TP
+.BI \-\-print-ckr-size
+Print crash kernel region size.
.SH SUPPORTED KERNEL FILE TYPES AND OPTIONS
diff --git a/kexec/kexec.c b/kexec/kexec.c
index cf6e03d..43c7e8c 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -960,6 +960,7 @@ void usage(void)
" --mem-max= Specify the highest memory address to\n"
" load code into.\n"
" --reuseinitrdReuse initrd from first boot.\n"
+ " --print-ckr-size Print crash kernel region size.\n"
" --load-preserve-context Load the new kernel and preserve\n"
" context of current kernel during kexec.\n"
" --load-jump-back-helper Load a helper image to jump back\n"
@@ -1345,6 +1346,9 @@ int main(int argc, char *argv[])
case OPT_KEXEC_FILE_SYSCALL:
/* We already parsed it. Nothing to do. */
break;
+ case OPT_PRINT_CKR_SIZE:
+ print_crashkernel_region_size();
+ return 0;
default:
break;
}
diff --git a/kexec/kexec.h b/kexec/kexec.h
index c02ac8f..42a602d 100644
--- a/kexec/kexec.h
+++ b/kexec/kexec.h
@@ -224,7 +224,8 @@ extern int file_types;
#define OPT_LOAD_PRESERVE_CONTEXT 259
#define OPT_LOAD_JUMP_BACK_HELPER 260
#define OPT_ENTRY 261
-#define OPT_MAX262
+#define OPT_PRINT_CKR_SIZE 262
+#define OPT_MAX263
#define KEXEC_OPTIONS \
{ "help", 0, 0, OPT_HELP }, \
{ "version",0, 0, OPT_VERSION }, \
@@ -244,6 +245,7 @@ extern int file_types;
{ "reuseinitrd",0, 0, OPT_REUSE_INITRD }, \
{ "kexec-file-syscall", 0, 0, OPT_KEXEC_FILE_SYSCALL }, \
[PATCH] x86/xen: Build memory type conversion code conditionally
Xen is not aware of E820_PMEM and E820_PRAM memory types, so, build simply fails. Hence, build code referencing them conditionally. Signed-off-by: Daniel Kiper --- kexec/arch/i386/kexec-x86-common.c |4 1 file changed, 4 insertions(+) diff --git a/kexec/arch/i386/kexec-x86-common.c b/kexec/arch/i386/kexec-x86-common.c index b308e47..21f8543 100644 --- a/kexec/arch/i386/kexec-x86-common.c +++ b/kexec/arch/i386/kexec-x86-common.c @@ -156,10 +156,14 @@ unsigned xen_e820_to_kexec_type(uint32_t type) return RANGE_ACPI; case E820_NVS: return RANGE_ACPI_NVS; +#ifdef E820_PMEM case E820_PMEM: return RANGE_PMEM; +#endif +#ifdef E820_PRAM case E820_PRAM: return RANGE_PRAM; +#endif case E820_RESERVED: default: return RANGE_RESERVED; -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] kexec-tools: Read always one vmcoreinfo file
Hi Petr, On Thu, Nov 13, 2014 at 04:51:48PM +0100, Petr Tesarik wrote: > Hi all, > > this thread got somehow forgotten because of vacations... > Anyway, read below. [...] Due to delays in EFI + GRUB2 + Xen project I must postpone work on this a bit longer than I expected. I will check what is going on immediately after releasing first version of patches for above mentioned project. It looks that it will happen at the beginning of next year. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] kexec-tools: Read always one vmcoreinfo file
Hi Petr, On Thu, Nov 13, 2014 at 04:51:48PM +0100, Petr Tesarik wrote: > Hi all, > > this thread got somehow forgotten because of vacations... > Anyway, read below. > > On Tue, 24 Jul 2012 15:54:10 +0200 > Daniel Kiper wrote: > > > On Tue, Jul 24, 2012 at 10:18:34AM +0200, Petr Tesarik wrote: > > > Dne Po 23. ??ervence 2012 22:10:59 Daniel Kiper napsal(a): > > > > Hi Petr, > > > > > > > > On Mon, Jul 23, 2012 at 03:30:55PM +0200, Petr Tesarik wrote: > > > > > Dne Po 23. ??ervence 2012 14:56:07 Petr Tesarik napsal(a): > > > > > > Dne ??t 5. ??ervence 2012 14:16:35 Daniel Kiper napsal(a): > > > > > > > vmcoreinfo file could exists under /sys/kernel (valid on baremetal > > > > > > > only) and/or under /sys/hypervisor (valid when Xen dom0 is > > > > > > > running). > > > > > > > Read only one of them. It means that only one PT_NOTE will be > > > > > > > always > > > > > > > created. Remove extra code for second PT_NOTE creation. > > > > > > > > > > > > Hi Daniel, > > > > > > > > > > > > are you absolutely sure this is the right thing to do? IIUC these > > > > > > two > > > > > > VMCORINFO notes are very different. The one from > > > > > > /sys/kernel/vmcoreinfo > > > > > > describes the Dom0 kernel (type 'VMCOREINFO'), while the one from > > > > > > /sys/hypervisor describes the Xen hypervisor (type > > > > > > 'XEN_VMCOREINFO'). > > > > > > If you keep only the hypervisor note, then e.g. makedumpfile won't > > > > > > be > > > > > > able to use dumplevel greater than 1, nor will it be able to extract > > > > > > the log buffer. > > > > > > > > > > I've just verified this, and I'm confident we have to keep both notes > > > > > in > > > > > the dump file. Simon, please revert Daniel's patch to avoid > > > > > regressions. > > > > > > > > > > I'm attaching a sample VMCOREINFO_XEN and VMCOREINFO to demonstrate > > > > > the > > > > > difference. Note that the VMCOREINFO_XEN note is actually too big, > > > > > because Xen doesn't bother to maintain the correct note size in the > > > > > note > > > > > header, so it always spans a complete page minus sizeof(Elf64_Nhdr)... > > > > > > > > [...] > > > > > > > > The problem with /sys/kernel/vmcoreinfo under Xen is that it expose > > > > invalid > > > > physical address. It breaks /proc/vmcore in crash kernel. That is why I > > > > proposed that fix. Additionally, /sys/kernel/vmcoreinfo is not available > > > > under Xen Linux Ver. 2.6.18. However, I did not do any makedumpfile > > > > tests. > > > > If you discovered any issues with my patch please drop me more details > > > > about your tests (Xen version, Linux Kernel version, makedumpfile > > > > version, > > > > command lines, config files, logs, etc.). I will be more then happy to > > > > fix/improve kexec-tools and makedumpfile. > > > > > > Hi Daniel, > > > > > > well, Linux v2.6.18 does not have /sys/kernel/vmcoreinfo, simply because > > > the > > > VMCOREINFO infrastructure was not present in 2.6.18. It was added later > > > with > > > > Yep. > > > > > commit fd59d231f81cb02870b9cf15f456a897f3669b4e, which went into 2.6.24. > > > > Hmmm... As I know 2.6.24 does not support kexec/kdump under Xen dom0. > > Correct? > > > > > I tested with the following combinations: > > > > > > * xen-3.3.1 + kernel-xen-2.6.27.54 + kexec-tools-2.0.0 + > > > makedumpfile-1.3.1 > > > * xen-4.0.3 + kernel-xen-2.6.32.59 + kexec-tools-2.0.0 + > > > makedumpfile-1.3.1 > > > * xen-4.1.2 + kernel-xen-3.0.34 + kexec-tools-2.0.0 + makedumpfile-1.4.0 > > > > > > These versions correspond to SLES11-GA, SLES11-SP1 and SLES11-SP2, > > > respectively. All of them work just fine and save both ELF notes into the > > > dump. > > > > Could you test current kexec-tools development version and > > latest makedumpfile version on latest SLES version? > > And indeed, I've just hit this regression with SLES12 GA (kernel 3.12.2
Re: [PATCH v2 3/4] makedumpfile/xen: Fail immediately on every architecture if dump level is invalid
On Fri, Aug 29, 2014 at 06:42:18AM +, Atsushi Kumagai wrote: > Hello Daniel, > > >On 2013/12/10 19:41:54, kexec wrote: > >> > > > Did you say that dump level 2 or larger are no longer effective even > >> > > > for x86_64 ? > >> > > > I thought it works by the patch below, but I'm not sure about Xen. > >> > > > So I would like to know why you sent this patch. > >> > > > > >> > > > > >> > > > commit ec5b5835a113cf62a168d4a7354564a38de6b52c > >> > > > Author: ken1_ohmichi > >> > > > Date: Fri Oct 9 03:05:41 2009 + > >> > > > > >> > > > [v1.3.4-10] Add dump filtering on an x86_64 xen domain-0. > >> > > > > >> > > > This patch adds the dump filtering for excluding unnecessary > >> > > > pages (cache > >> > > > pages, user process data pages, and free pages) on on x86_64 xen > >> > > > domain-0. > >> > > > > >> > > > On the existing makedumpfile (v1.3.3 or former), a user could > >> > > > specify 0 > >> > > > or 1 only as a dump_level. By this patch, he/she can specify 2 > >> > > > or larger > >> > > > also as a dump_level. > >> > > > > >> > > > Now, this feature is effective on x86_64 machine only. > >> > > > >> > > Hmmm... Thanks for this. I missed this patch. However, it looks that I > >> > > do not understand something. AIUI, from Xen point of view we are not > >> > > able > >> > > to use dump level higher than 1 because there is no e.g. cache pages > >> > > (it > >> > > looks that we could also skip free pages but this stuff is not > >> > > implemented). > >> > > Above mentioned patch suggest that there is a way to extract just only > >> > > Dom0 > >> > > stuff taking into account Linux internals only. If my reasoning is true > >> > > then dump level higher than 1 is possible only if we look at Dom0 from > >> > > Linux > >> > > point of view. > >> > > >> > I've reviewed the code for Xen, my understanding is the same as yours. > >> > The memory regions corresponding to hypervisor and DomU will remain even > >> > if > >> > specifying the dump level higher than 1. > >> > > >> > > However, I can not find any description how to do that. > >> > > So I am CC-ing Ken'ichi as author of this patch but I do not know that > >> > > he works for NEC still. > >> > > >> > I'm sorry but I missed your point. Did you mention a lack of description > >> > in man page about an effect when specifying the dump level higher than 1 > >> > for Xen's memory ? > >> > At least, I still think this patch is wrong because any dump level is > >> > effective for x86_64. > >> > >> Docs are not consistent because man and help displayed from makedumpfile > >> are different. Additionally, even man says nothing how to use this feature > >> on Xen vmcore file. If you use makedumpfile e.g. > >> > >> makedumpfile -Ed 2 /proc/vmcore vmcore > >> > >> it will not work because it uses VMCOREINFO_XEN instead of VMCOREINFO. > >> I discovered that if you would like to use feature from above mentioned > >> patch you must run makedumpfile in following way: > >> > >> makedumpfile -Ed 2 -x vmlinux /proc/vmcore vmcore > >> > >> Then makedumpfile will get info about dom0 directly from vmlinux. > > > >Certainly the documents should be fixed as you said, I'll do it. > > Sorry for my too late job, how about this fix ? No problem. I am busy too. I still have patch fixing this feature for new Xen versions on my long todo list. I hope that I will be able to work on it at the beginning of next year. > I'll merge it into v1.5.7 if you have no objection. Great! Thanks! > Thanks > Atsushi Kumagai > > From: Atsushi Kumagai > Date: Fri, 29 Aug 2014 15:11:27 +0900 > Subject: [PATCH] Fix description about filtering for Xen. > > Correct the following points for Xen dump: > > - Add how to filter the pages of domain-0. > - Fix the difference between man and help. > > Reported-by: Daniel Kiper > Signed-off-by: Atsushi Kumagai Reviewed-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 0/4] Enable use of crash on xen 4.4.0 vmcore
On Sat, Jan 04, 2014 at 02:11:22PM -0500, Don Slutz wrote: > With the addition on PVH code to xen 4.4, domain.is_hvm no longer > exists. This prevents crash from using a xen 4.4.0 vmcore. > > Patch 1 "fixes" this. > > Patch 2 is a minor fix in that outputing the offset in hex for > domain_domain_flags is different. > > Patch 3 is a bug fix to get all "domain_flags" set, not just the 1st > one found. > > Patch 4 is a quick way to add domain.guest_type support. Sorry for late reply but I was on holiday. Nice work. Thanks. I have done some tests with old and new Xen versions and it looks that it works without any issue. Reviewed-by: Daniel Kiper Tested-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 3/4] makedumpfile/xen: Fail immediately on every architecture if dump level is invalid
On Fri, Dec 13, 2013 at 06:59:06AM +, Atsushi Kumagai wrote: > On 2013/12/10 19:41:54, kexec wrote: [...] > > Docs are not consistent because man and help displayed from makedumpfile > > are different. Additionally, even man says nothing how to use this feature > > on Xen vmcore file. If you use makedumpfile e.g. > > > > makedumpfile -Ed 2 /proc/vmcore vmcore > > > > it will not work because it uses VMCOREINFO_XEN instead of VMCOREINFO. > > I discovered that if you would like to use feature from above mentioned > > patch you must run makedumpfile in following way: > > > > makedumpfile -Ed 2 -x vmlinux /proc/vmcore vmcore > > > > Then makedumpfile will get info about dom0 directly from vmlinux. > > Certainly the documents should be fixed as you said, I'll do it. Thanks. > > However, It looks that there is another bug which prevents usage > > of this feature. It looks it is related to change in P2M tree Linux > > code. Once P2M levels where changed from 2 to 3. I fixed similar > > issue in crash tool once. > > Thanks for your pointing out, I hope that you will fix that issue > also in makedumpfile. Yes, I am going to do that but not now. I am just very busy. Additionally, I have discovered that this feature code is broken in more places because it was not maintained more than four years. So, to be honest, it is simply unusable right now. > > When are you going to make a new makedumpfile release? I am going > > to fix this issue before next release but now I am quite busy > > with other stuff. > > I must release the new version(v1.5.5) in the next week at the latest > because I announced the release date will be the beginning of December > and there are already many patches for v1.5.5. > So I would like to slip that fix you mentioned to v1.5.6. OK. > > Could you apply patches 1, 2 and 4 from this patch series? > > It looks that they are not controversial. > > Sure, the three patches will be merged into v1.5.5. Thanks. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 3/4] makedumpfile/xen: Fail immediately on every architecture if dump level is invalid
On Mon, Dec 09, 2013 at 02:45:21AM +, Atsushi Kumagai wrote:
> On 2013/12/04 4:45:42, kexec wrote:
> > On Tue, Dec 03, 2013 at 05:27:03AM +, Atsushi Kumagai wrote:
> > > On 2013/12/02 23:17:59, kexec wrote:
> > > > Do not try to process Xen crash dump on every architecture if dump level
> > > > is invalid. Fail immediately and print relevant error message.
> > > >
> > > > Signed-off-by: Daniel Kiper
> > > > ---
> > > > makedumpfile.c |4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/makedumpfile.c b/makedumpfile.c
> > > > index 5a378d1..45f96aa 100644
> > > > --- a/makedumpfile.c
> > > > +++ b/makedumpfile.c
> > > > @@ -7637,14 +7637,14 @@ initial_xen(void)
> > > > MSG("Try `makedumpfile --help' for more
> > > > information.\n");
> > > > return FALSE;
> > > > }
> > > > -#ifndef __x86_64__
> > > > +
> > > > if (DL_EXCLUDE_ZERO < info->max_dump_level) {
> > > > MSG("Dump_level is invalid. It should be 0 or 1.\n");
> > > > MSG("Commandline parameter is invalid.\n");
> > > > MSG("Try `makedumpfile --help' for more
> > > > information.\n");
> > > > return FALSE;
> > > > }
> > > > -#endif
> > > > +
> > >
> > > Did you say that dump level 2 or larger are no longer effective even for
> > > x86_64 ?
> > > I thought it works by the patch below, but I'm not sure about Xen.
> > > So I would like to know why you sent this patch.
> > >
> > >
> > > commit ec5b5835a113cf62a168d4a7354564a38de6b52c
> > > Author: ken1_ohmichi
> > > Date: Fri Oct 9 03:05:41 2009 +
> > >
> > > [v1.3.4-10] Add dump filtering on an x86_64 xen domain-0.
> > >
> > > This patch adds the dump filtering for excluding unnecessary pages
> > > (cache
> > > pages, user process data pages, and free pages) on on x86_64 xen
> > > domain-0.
> > >
> > > On the existing makedumpfile (v1.3.3 or former), a user could specify > > > 0
> > > or 1 only as a dump_level. By this patch, he/she can specify 2 or
> > > larger
> > > also as a dump_level.
> > >
> > > Now, this feature is effective on x86_64 machine only.
> >
> > Hmmm... Thanks for this. I missed this patch. However, it looks that I
> > do not understand something. AIUI, from Xen point of view we are not able
> > to use dump level higher than 1 because there is no e.g. cache pages (it
> > looks that we could also skip free pages but this stuff is not implemented).
> > Above mentioned patch suggest that there is a way to extract just only Dom0
> > stuff taking into account Linux internals only. If my reasoning is true
> > then dump level higher than 1 is possible only if we look at Dom0 from Linux
> > point of view.
>
> I've reviewed the code for Xen, my understanding is the same as yours.
> The memory regions corresponding to hypervisor and DomU will remain even if
> specifying the dump level higher than 1.
>
> > However, I can not find any description how to do that.
> > So I am CC-ing Ken'ichi as author of this patch but I do not know that
> > he works for NEC still.
>
> I'm sorry but I missed your point. Did you mention a lack of description
> in man page about an effect when specifying the dump level higher than 1
> for Xen's memory ?
> At least, I still think this patch is wrong because any dump level is
> effective for x86_64.
Docs are not consistent because man and help displayed from makedumpfile
are different. Additionally, even man says nothing how to use this feature
on Xen vmcore file. If you use makedumpfile e.g.
makedumpfile -Ed 2 /proc/vmcore vmcore
it will not work because it uses VMCOREINFO_XEN instead of VMCOREINFO.
I discovered that if you would like to use feature from above mentioned
patch you must run makedumpfile in following way:
makedumpfile -Ed 2 -x vmlinux /proc/vmcore vmcore
Then makedumpfile will get info about dom0 directly from vmlinux.
However, It looks that there is another bug which prevents usage
of this feature. It looks it is related to change in P2M tree Linux
code. Once P2M levels where changed from 2 to 3. I fixed similar
issue in crash tool once.
When are you going to make a new makedumpfile release? I am going
to fix this issue before next release but now I am quite busy
with other stuff.
Could you apply patches 1, 2 and 4 from this patch series?
It looks that they are not controversial.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v2 3/4] makedumpfile/xen: Fail immediately on every architecture if dump level is invalid
On Tue, Dec 03, 2013 at 05:27:03AM +, Atsushi Kumagai wrote:
> On 2013/12/02 23:17:59, kexec wrote:
> > Do not try to process Xen crash dump on every architecture if dump level
> > is invalid. Fail immediately and print relevant error message.
> >
> > Signed-off-by: Daniel Kiper
> > ---
> > makedumpfile.c |4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index 5a378d1..45f96aa 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -7637,14 +7637,14 @@ initial_xen(void)
> > MSG("Try `makedumpfile --help' for more information.\n");
> > return FALSE;
> > }
> > -#ifndef __x86_64__
> > +
> > if (DL_EXCLUDE_ZERO < info->max_dump_level) {
> > MSG("Dump_level is invalid. It should be 0 or 1.\n");
> > MSG("Commandline parameter is invalid.\n");
> > MSG("Try `makedumpfile --help' for more information.\n");
> > return FALSE;
> > }
> > -#endif
> > +
>
> Did you say that dump level 2 or larger are no longer effective even for
> x86_64 ?
> I thought it works by the patch below, but I'm not sure about Xen.
> So I would like to know why you sent this patch.
>
>
> commit ec5b5835a113cf62a168d4a7354564a38de6b52c
> Author: ken1_ohmichi
> Date: Fri Oct 9 03:05:41 2009 +
>
> [v1.3.4-10] Add dump filtering on an x86_64 xen domain-0.
>
> This patch adds the dump filtering for excluding unnecessary pages (cache
> pages, user process data pages, and free pages) on on x86_64 xen domain-0.
>
> On the existing makedumpfile (v1.3.3 or former), a user could specify 0
> or 1 only as a dump_level. By this patch, he/she can specify 2 or larger
> also as a dump_level.
>
> Now, this feature is effective on x86_64 machine only.
Hmmm... Thanks for this. I missed this patch. However, it looks that I
do not understand something. AIUI, from Xen point of view we are not able
to use dump level higher than 1 because there is no e.g. cache pages (it
looks that we could also skip free pages but this stuff is not implemented).
Above mentioned patch suggest that there is a way to extract just only Dom0
stuff taking into account Linux internals only. If my reasoning is true
then dump level higher than 1 is possible only if we look at Dom0 from Linux
point of view. However, I can not find any description how to do that.
So I am CC-ing Ken'ichi as author of this patch but I do not know that
he works for NEC still.
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/2] kexec-tools/xen: Do not call xc_interface_close() if xc_interface_open() failed
Do not call xc_interface_close() if xc_interface_open() failed.
xc_interface_close() crashes if it gets NULL as an argument.
Relevant fix for libxenctrl will be posted too but kexec-tools
should also behave properly.
Signed-off-by: Daniel Kiper
---
kexec/arch/i386/crashdump-x86.c|2 +-
kexec/arch/i386/kexec-x86-common.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
index 85f3a87..a3eb5ae 100644
--- a/kexec/arch/i386/crashdump-x86.c
+++ b/kexec/arch/i386/crashdump-x86.c
@@ -305,7 +305,7 @@ static int get_crash_memory_ranges_xen(struct memory_range
**range,
if (!xc) {
fprintf(stderr, "%s: Failed to open Xen control interface\n",
__func__);
- goto err;
+ return -1;
}
rc = xc_get_machine_memory_map(xc, e820entries,
CRASH_MAX_MEMORY_RANGES);
diff --git a/kexec/arch/i386/kexec-x86-common.c
b/kexec/arch/i386/kexec-x86-common.c
index bf58f53..f55e2c2 100644
--- a/kexec/arch/i386/kexec-x86-common.c
+++ b/kexec/arch/i386/kexec-x86-common.c
@@ -176,7 +176,7 @@ static int get_memory_ranges_xen(struct memory_range
**range, int *ranges)
if (!xc) {
fprintf(stderr, "%s: Failed to open Xen control interface\n",
__func__);
- goto err;
+ return -1;
}
rc = xc_get_machine_memory_map(xc, e820entries, MAX_MEMORY_RANGES);
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/2] kexec-tools: Minor Xen fixes and cleanups
Hi, This patch series contains following minor Xen fixes and cleanups for kexec-tools: - kexec-tools/xen: Do not call xc_interface_close() if xc_interface_open() failed, - kexec-tools/xen: Remove redundant include. Please apply. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 2/2] kexec-tools/xen: Remove redundant include
Remove redundant include of stdlib.h introduced by commit d01680c28ba7bf1e02f74aa5841247a45738f5d4 (kexec/xen: Correct some compile errors). Signed-off-by: Daniel Kiper --- kexec/kexec-xen.c |1 - 1 file changed, 1 deletion(-) diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c index e885246..cac313d 100644 --- a/kexec/kexec-xen.c +++ b/kexec/kexec-xen.c @@ -2,7 +2,6 @@ #include #include #include -#include #include #include "kexec.h" #include "kexec-syscall.h" -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 0/4] makedumpfile: Xen fixes and cleanups
Hi, This is second version of patch series containing following makedumpfile Xen fixes and cleanups: - makedumpfile/xen: Add cache_init() call to initial_xen(), - makedumpfile/xen: Disable cyclic mode for every Xen crash dump, - makedumpfile/xen: Fail immediately on every architecture if dump level is invalid, - makedumpfile/xen: Move cyclic mode check from initial() to initial_xen(). All patches were tested with Xen versions up to latest unstable. Please apply. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 3/4] makedumpfile/xen: Fail immediately on every architecture if dump level is invalid
Do not try to process Xen crash dump on every architecture if dump level
is invalid. Fail immediately and print relevant error message.
Signed-off-by: Daniel Kiper
---
makedumpfile.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 5a378d1..45f96aa 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7637,14 +7637,14 @@ initial_xen(void)
MSG("Try `makedumpfile --help' for more information.\n");
return FALSE;
}
-#ifndef __x86_64__
+
if (DL_EXCLUDE_ZERO < info->max_dump_level) {
MSG("Dump_level is invalid. It should be 0 or 1.\n");
MSG("Commandline parameter is invalid.\n");
MSG("Try `makedumpfile --help' for more information.\n");
return FALSE;
}
-#endif
+
if (!init_xen_crash_info())
return FALSE;
/*
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 4/4] makedumpfile/xen: Move cyclic mode check from initial() to initial_xen()
Move cyclic mode check from initial() to initial_xen(). Just small cleanup.
Signed-off-by: Daniel Kiper
---
makedumpfile.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 45f96aa..e173f85 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2928,14 +2928,6 @@ initial(void)
}
#endif
- if (is_xen_memory()) {
- if(info->flag_cyclic) {
- info->flag_cyclic = FALSE;
- MSG("Switched running mode from cyclic to
non-cyclic,\n");
- MSG("because the cyclic mode doesn't support Xen.\n");
- }
- }
-
if (info->flag_exclude_xen_dom && !is_xen_memory()) {
MSG("'-X' option is disable,");
MSG("because %s is not Xen's memory core image.\n",
info->name_memory);
@@ -7645,6 +7637,12 @@ initial_xen(void)
return FALSE;
}
+ if(info->flag_cyclic) {
+ info->flag_cyclic = FALSE;
+ MSG("Switched running mode from cyclic to non-cyclic,\n");
+ MSG("because the cyclic mode doesn't support Xen.\n");
+ }
+
if (!init_xen_crash_info())
return FALSE;
/*
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 2/4] makedumpfile/xen: Disable cyclic mode for every Xen crash dump
Disable cyclic mode for every Xen crash dump not only when -X option
is used. This way makedumpfile is much more flexible and different
modes could be used to dump whole system memory (e.g. zero pages
could be striped without requiring -X option).
Signed-off-by: Daniel Kiper
---
makedumpfile.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 55e53b7..5a378d1 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2928,20 +2928,20 @@ initial(void)
}
#endif
- if (info->flag_exclude_xen_dom) {
+ if (is_xen_memory()) {
if(info->flag_cyclic) {
info->flag_cyclic = FALSE;
MSG("Switched running mode from cyclic to
non-cyclic,\n");
MSG("because the cyclic mode doesn't support Xen.\n");
}
+ }
- if (!is_xen_memory()) {
- MSG("'-X' option is disable,");
- MSG("because %s is not Xen's memory core image.\n",
info->name_memory);
- MSG("Commandline parameter is invalid.\n");
- MSG("Try `makedumpfile --help' for more
information.\n");
- return FALSE;
- }
+ if (info->flag_exclude_xen_dom && !is_xen_memory()) {
+ MSG("'-X' option is disable,");
+ MSG("because %s is not Xen's memory core image.\n",
info->name_memory);
+ MSG("Commandline parameter is invalid.\n");
+ MSG("Try `makedumpfile --help' for more information.\n");
+ return FALSE;
}
if (info->flag_refiltering) {
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2 1/4] makedumpfile/xen: Add cache_init() call to initial_xen()
Commit 92563d7a7a5175ef78c4a94ee269b1b455331b4c (cache: Allocate buffers
at initialization to detect malloc() failure) split cache_alloc() to
cache_init() and cache_alloc(). cache_init() is called in initial().
However, sadly initial_xen() is called before cache_init() and it uses
cache_alloc() indirectly which refers to uninitialized structures.
Hence, makedumfile run on Xen crash dumps finally fails. This patch adds
cache_init() call to initial_xen() and fixes above mentioned issue.
cache_init() is called in initial() if crash dump is not Xen one.
Signed-off-by: Daniel Kiper
---
makedumpfile.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 0c68f32..55e53b7 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3117,7 +3117,7 @@ out:
DEBUG_MSG("Buffer size for the cyclic mode: %ld\n",
info->bufsize_cyclic);
}
- if (!cache_init())
+ if (!is_xen_memory() && !cache_init())
return FALSE;
if (debug_info) {
@@ -7622,6 +7622,7 @@ exclude_xen_user_domain(void)
int
initial_xen(void)
{
+ int xen_info_required = TRUE;
off_t offset;
unsigned long size;
@@ -7673,8 +7674,10 @@ initial_xen(void)
* and get both the offset and the size.
*/
if (!has_vmcoreinfo_xen()){
- if (!info->flag_exclude_xen_dom)
+ if (!info->flag_exclude_xen_dom) {
+ xen_info_required = FALSE;
goto out;
+ }
MSG("%s doesn't contain a vmcoreinfo for Xen.\n",
info->name_memory);
@@ -7691,6 +7694,7 @@ initial_xen(void)
return FALSE;
}
+out:
if (!info->page_size) {
/*
* If we cannot get page_size from a vmcoreinfo file,
@@ -7700,12 +7704,17 @@ initial_xen(void)
return FALSE;
}
- if (!get_xen_info())
+ if (!cache_init())
return FALSE;
- if (message_level & ML_PRINT_DEBUG_MSG)
- show_data_xen();
-out:
+ if (xen_info_required == TRUE) {
+ if (!get_xen_info())
+ return FALSE;
+
+ if (message_level & ML_PRINT_DEBUG_MSG)
+ show_data_xen();
+ }
+
if (!get_max_mapnr())
return FALSE;
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 1/3] makedumpfile/xen: Move Xen crash dump check
Commit 92563d7a7a5175ef78c4a94ee269b1b455331b4c (cache: Allocate buffers at initialization to detect malloc() failure) split cache_alloc() to cache_init() and cache_alloc(). cache_init() is called in initial(). However, sadly initial_xen() is called before cache_init() and it uses cache_alloc() indirectly which refers to uninitialized structures. Hence, makedumfile run on Xen crash dumps finally fails. This patch moves initial_xen() behind cache_init() call and fixes above mentioned issue. Signed-off-by: Daniel Kiper --- makedumpfile.c |5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 0c68f32..74376cd 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -2905,9 +2905,6 @@ initial(void) unsigned long size; int debug_info = FALSE; - if (is_xen_memory() && !initial_xen()) - return FALSE; - #ifdef USELZO if (lzo_init() == LZO_E_OK) info->flag_lzo_support = TRUE; @@ -3172,7 +3169,7 @@ out: return FALSE; } - if (is_xen_memory() && !get_dom0_mapnr()) + if (is_xen_memory() && (!initial_xen() || !get_dom0_mapnr())) return FALSE; if (!get_value_for_old_linux()) -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 0/3] makedumpfile: Xen fixes
Hi, This patch series contains following makedumpfile Xen fixes: - makedumpfile/xen: Move Xen crash dump check, - makedumpfile/xen: Disable cyclic mode for every Xen crash dump, - makedumpfile/xen: Fail immediately if dump level is invalid. All patches were tested with Xen versions up to latest unstable. Please apply. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 3/3] makedumpfile/xen: Fail immediately if dump level is invalid
Do not try to process Xen crash dump if dump level is invalid.
Fail immediately and print relevant error message.
Signed-off-by: Daniel Kiper
---
makedumpfile.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index b160cea..389efc5 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -64,6 +64,8 @@ do { \
static void check_cyclic_buffer_overrun(void);
static void setup_page_is_buddy(void);
+int get_next_dump_level(int index);
+
void
initialize_tables(void)
{
@@ -2903,7 +2905,7 @@ initial(void)
{
off_t offset;
unsigned long size;
- int debug_info = FALSE;
+ int debug_info = FALSE, dump_level, i;
#ifdef USELZO
if (lzo_init() == LZO_E_OK)
@@ -2926,6 +2928,20 @@ initial(void)
#endif
if (is_xen_memory()) {
+ for (i = 0; ; ++i) {
+ dump_level = get_next_dump_level(i);
+
+ if (dump_level < 0)
+ break;
+
+ if (dump_level <= 1)
+ continue;
+
+ MSG("Allowed Dump_Level for Xen dump filtering is 0 or
1\n");
+
+ return FALSE;
+ }
+
if(info->flag_cyclic) {
info->flag_cyclic = FALSE;
MSG("Switched running mode from cyclic to
non-cyclic,\n");
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCH 2/3] makedumpfile/xen: Disable cyclic mode for every Xen crash dump
Disable cyclic mode for every Xen crash dump not only when -X option
is used. This way makedumpfile is much more flexible and different
modes could be used to dump whole system memory (e.g. zero pages
could be striped without requiring -X option).
Signed-off-by: Daniel Kiper
---
makedumpfile.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 74376cd..b160cea 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2925,20 +2925,20 @@ initial(void)
}
#endif
- if (info->flag_exclude_xen_dom) {
+ if (is_xen_memory()) {
if(info->flag_cyclic) {
info->flag_cyclic = FALSE;
MSG("Switched running mode from cyclic to
non-cyclic,\n");
MSG("because the cyclic mode doesn't support Xen.\n");
}
+ }
- if (!is_xen_memory()) {
- MSG("'-X' option is disable,");
- MSG("because %s is not Xen's memory core image.\n",
info->name_memory);
- MSG("Commandline parameter is invalid.\n");
- MSG("Try `makedumpfile --help' for more
information.\n");
- return FALSE;
- }
+ if (info->flag_exclude_xen_dom && !is_xen_memory()) {
+ MSG("'-X' option is disable,");
+ MSG("because %s is not Xen's memory core image.\n",
info->name_memory);
+ MSG("Commandline parameter is invalid.\n");
+ MSG("Try `makedumpfile --help' for more information.\n");
+ return FALSE;
}
if (info->flag_refiltering) {
--
1.7.10.4
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] kexec/xen: Fix typo in __i386__ preprocessor identifier
On Tue, Nov 26, 2013 at 05:42:37PM +, Andrew Cooper wrote: > This typo was introduced in c/s c0b4a3f95dd80256cc6d7084436235e69b4933fb > > It prevents a 32bit build of kexec from loading a 32bit crash image. > > Signed-off-by: Andrew Cooper > CC: David Vrabel > CC: Daniel Kiper > CC: Simon Horman > CC: kexec@lists.infradead.org Reviewed-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 4/4] kexec/xen: directly load images images into Xen
On Tue, Nov 19, 2013 at 10:20:14AM +0900, Simon Horman wrote:
> On Wed, Nov 06, 2013 at 02:55:22PM +, David Vrabel wrote:
> > From: David Vrabel
> >
> > Xen 4.4 has an improvided kexec hypercall ABI that allows images to be
> > loaded and executed without any kernel involvement. Use the API
> > provided by libxc to load images when running in a Xen guest.
> >
> > Support for loading images via the kexec_load syscall in non-upstream
> > ("classic") Xen kernels is no longer supported.
> >
> > Signed-off-by: David Vrabel
> > Reviewed-by: Daniel Kiper
> > ---
> > kexec/Makefile |1 +
> > kexec/arch/i386/crashdump-x86.c | 20 +-
> > kexec/crashdump-xen.c | 34 ++
> > kexec/crashdump.h |3 +-
> > kexec/kexec-xen.c | 139
> > +++
> > kexec/kexec.c | 24 +--
> > kexec/kexec.h |5 ++
> > 7 files changed, 218 insertions(+), 8 deletions(-)
> > create mode 100644 kexec/kexec-xen.c
> >
[...]
> I have applied the following follow-up patch to resolves some errors.
> Let me know if I messed it up.
>
> From: Simon Horman
>
> [PATCH] kexec/xen: Correct some compile errors
>
> Correct various problems introduced by
> 08cf823704b0fa3b ("kexec/xen: directly load images images into Xen").
>
> These all relate to the case here HAVE_LIBXENCTRL is not set.
>
> Signed-off-by: Simon Horman
> ---
> kexec/kexec-xen.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 77f65c0..e885246 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -2,6 +2,7 @@
> #include
> #include
> #include
> +#include
Thanks for fixes but why are you including stdlib.h twice?
Daniel
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image
On Mon, Nov 18, 2013 at 12:05:38PM +, George Dunlap wrote: > On 18/11/13 11:47, Daniel Kiper wrote: > >On Mon, Nov 18, 2013 at 11:23:27AM +, David Vrabel wrote: > >>On 18/11/13 09:29, Jan Beulich wrote: > >>>>>>On 15.11.13 at 21:07, David Vrabel wrote: > >>>>On 15/11/13 15:56, Daniel Kiper wrote: > >>>>>Clear unused registers before jumping into an image. This way > >>>>>loaded image could not assume that any register has an specific > >>>>>info about earlier running Xen hypervisor. However, it also > >>>>>does not mean that the image may expect that a given register > >>>>>is zeroed. The image MUST assume that every register has a random > >>>>>value or in other words it is uninitialized or has undefined state. > >>>>I think this, where the specification (registers undefined) differs from > >>>>the implementation (registers zeroed), is the worst option. > >>>> > >>>>I also think it is more likely for an image to inadvertently rely on a > >>>>zero value that whatever junk Xen has left behind. > >>>Preventing users to rely on anything would likely make it > >>>desirable to put some random value into all unused registers. > >>I don't think we need to go that far. > >> > >>I would just like to avoid someone looking that the implementation (and > >>not the documentation) and concluding that zero-ing of the registers is > >>part of the specified behaviour, or looking at the implementation and > >>documentation and wondering why they don't agree. > >David, my comment clearly states why we are doing that and what should > >be expected. What is wrong with it? I could improve it but say how? > > You seem to be saying that the registers may contain useful > information about Xen that are not part of the spec, and you are > worried that the image may decide to use these and come to rely on > them, making it hard to change the interface in the future. > > David has a similar concern -- that if the registers are zeroed, the > image may come to rely on the registers being pre-zeroed, and not > zero them itself. This would also make it hard to change the > interface in the future. > > A simple solution would be to "poison" the registers with useless > data: 0xdeadbeef is a favorite. Anyone seeing that in the registers > will immediately know, "Someone used a value that they shouldn't > have." > > Of course, it's *possible* that then the images might come to rely > on that poison being there; having a non-deterministic value there, > like a hash of the TSC, would make this impossible. But even then, > you could make the argument that the image may come to rely on a > *pseudo-random* number being there, which it uses for some other > kind of seed somewhere. At some point you just have to give up on > this like of thought. :-) :-))) > Personally I think having a poison is likely to be more useful -- if > you crash because your pointer is 0xdeadbeef, then it's immediately > obvious what kind of bug you have; whereas if you crash with a > random value that changes every time, it's not so obvious. It works for me too. Any solution has pros and cons. However, in general I think that wiping registers in that case is nice idea. So we could zero registers, write 0xdeadbeef into them or even use TSC. But please do not leave registers as is. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image
On Mon, Nov 18, 2013 at 11:27:34AM +, Jan Beulich wrote: > >>> On 18.11.13 at 12:08, Daniel Kiper wrote: > > On Mon, Nov 18, 2013 at 09:29:41AM +, Jan Beulich wrote: > >> >>> On 15.11.13 at 21:07, David Vrabel wrote: > >> > On 15/11/13 15:56, Daniel Kiper wrote: > >> >> Clear unused registers before jumping into an image. This way > >> >> loaded image could not assume that any register has an specific > >> >> info about earlier running Xen hypervisor. However, it also > >> >> does not mean that the image may expect that a given register > >> >> is zeroed. The image MUST assume that every register has a random > >> >> value or in other words it is uninitialized or has undefined state. > >> > > >> > I think this, where the specification (registers undefined) differs from > >> > the implementation (registers zeroed), is the worst option. > >> > > >> > I also think it is more likely for an image to inadvertently rely on a > >> > zero value that whatever junk Xen has left behind. > >> > >> Preventing users to rely on anything would likely make it > >> desirable to put some random value into all unused registers. > > > > Right, but on the other hand this way we lose completely chance > > to differentiate between old and new implementation of kexec > > if we would like to do that in the future (yes, this is small > > chance but it still exists). Additionally, I think it could be > > quite difficult because at this stage there is no simple reliable > > RNGs. Although there are some CPUs with RNGs but they are not > > very common right now. However, I will do not object if we find > > another simple RNG. > > We surely wouldn't need a good quality random number here - > the TSC would very likely already be more random than anything > we need. I forgot about TSC. This is OK in that case. Thanks. Personally I prefer zeroing (I explained above and in other emails why) but if David do not like it we could use TSC. David? Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image
On Mon, Nov 18, 2013 at 11:23:27AM +, David Vrabel wrote: > On 18/11/13 09:29, Jan Beulich wrote: > >>>> On 15.11.13 at 21:07, David Vrabel wrote: > >> On 15/11/13 15:56, Daniel Kiper wrote: > >>> Clear unused registers before jumping into an image. This way > >>> loaded image could not assume that any register has an specific > >>> info about earlier running Xen hypervisor. However, it also > >>> does not mean that the image may expect that a given register > >>> is zeroed. The image MUST assume that every register has a random > >>> value or in other words it is uninitialized or has undefined state. > >> > >> I think this, where the specification (registers undefined) differs from > >> the implementation (registers zeroed), is the worst option. > >> > >> I also think it is more likely for an image to inadvertently rely on a > >> zero value that whatever junk Xen has left behind. > > > > Preventing users to rely on anything would likely make it > > desirable to put some random value into all unused registers. > > I don't think we need to go that far. > > I would just like to avoid someone looking that the implementation (and > not the documentation) and concluding that zero-ing of the registers is > part of the specified behaviour, or looking at the implementation and > documentation and wondering why they don't agree. David, my comment clearly states why we are doing that and what should be expected. What is wrong with it? I could improve it but say how? Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image
On Mon, Nov 18, 2013 at 09:29:41AM +, Jan Beulich wrote: > >>> On 15.11.13 at 21:07, David Vrabel wrote: > > On 15/11/13 15:56, Daniel Kiper wrote: > >> Clear unused registers before jumping into an image. This way > >> loaded image could not assume that any register has an specific > >> info about earlier running Xen hypervisor. However, it also > >> does not mean that the image may expect that a given register > >> is zeroed. The image MUST assume that every register has a random > >> value or in other words it is uninitialized or has undefined state. > > > > I think this, where the specification (registers undefined) differs from > > the implementation (registers zeroed), is the worst option. > > > > I also think it is more likely for an image to inadvertently rely on a > > zero value that whatever junk Xen has left behind. > > Preventing users to rely on anything would likely make it > desirable to put some random value into all unused registers. Right, but on the other hand this way we lose completely chance to differentiate between old and new implementation of kexec if we would like to do that in the future (yes, this is small chance but it still exists). Additionally, I think it could be quite difficult because at this stage there is no simple reliable RNGs. Although there are some CPUs with RNGs but they are not very common right now. However, I will do not object if we find another simple RNG. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] xen/kexec: Clear unused registers before jumping into an image
On Fri, Nov 15, 2013 at 08:07:02PM +, David Vrabel wrote: > On 15/11/13 15:56, Daniel Kiper wrote: > > Clear unused registers before jumping into an image. This way > > loaded image could not assume that any register has an specific > > info about earlier running Xen hypervisor. However, it also > > does not mean that the image may expect that a given register > > is zeroed. The image MUST assume that every register has a random > > value or in other words it is uninitialized or has undefined state. > > I think this, where the specification (registers undefined) differs from > the implementation (registers zeroed), is the worst option. > > I also think it is more likely for an image to inadvertently rely on a > zero value that whatever junk Xen has left behind. I do not agree with you but respect your opinion. So could you provide a patch with a comment why our implementation deviate from our reference implementation (I think about Linux one) and even we use kexec-tools designed for Linux implementation which does things mentioned above? I hope that this solve this last, widely discussed issue. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH] xen/kexec: Clear unused registers before jumping into an image
Clear unused registers before jumping into an image. This way loaded image could not assume that any register has an specific info about earlier running Xen hypervisor. However, it also does not mean that the image may expect that a given register is zeroed. The image MUST assume that every register has a random value or in other words it is uninitialized or has undefined state. Signed-off-by: Daniel Kiper --- xen/arch/x86/x86_64/kexec_reloc.S | 37 + 1 file changed, 37 insertions(+) diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S index 7a16c85..e7eef79 100644 --- a/xen/arch/x86/x86_64/kexec_reloc.S +++ b/xen/arch/x86/x86_64/kexec_reloc.S @@ -71,6 +71,29 @@ identity_mapped: jnz call_32_bit call_64_bit: +/* + * Clear unused registers before jumping into an image. This way + * loaded image could not assume that any register has an specific + * info about earlier running Xen hypervisor. However, it also + * does not mean that the image may expect that a given register + * is zeroed. The image MUST assume that every register has a random + * value or in other words it is uninitialized or has undefined state. + */ +xorl%eax, %eax +xorl%ebx, %ebx +xorl%ecx, %ecx +xorl%edx, %edx +xorl%esi, %esi +xorl%edi, %edi +xorl%r8d, %r8d +xorl%r9d, %r9d +xorl%r10d, %r10d +xorl%r11d, %r11d +xorl%r12d, %r12d +xorl%r13d, %r13d +xorl%r14d, %r14d +xorl%r15d, %r15d + /* Call the image entry point. This should never return. */ callq *%rbp ud2 @@ -164,6 +187,20 @@ compatibility_mode: xorl%eax, %eax movl%eax, %cr4 +/* + * Clear unused registers before jumping into an image. This way + * loaded image could not assume that any register has an specific + * info about earlier running Xen hypervisor. However, it also + * does not mean that the image may expect that a given register + * is zeroed. The image MUST assume that every register has a random + * value or in other words it is uninitialized or has undefined state. + */ +xorl%ebx, %ebx +xorl%ecx, %ecx +xorl%edx, %edx +xorl%esi, %esi +xorl%edi, %edi + /* Call the image entry point. This should never return. */ call*%ebp ud2 -- 1.7.10.4 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv7 0/4] kexec-tools: add support for Xen 4.4
On Wed, Nov 06, 2013 at 02:55:18PM +, David Vrabel wrote: > The series adds support for the new hypercall ABI which should be > provided by Xen 4.4. Images are loaded into Xen directly with no > kernel involvement. > > Please do not apply until the hypervisor side patches are applied to > Xen. > > Patch 1 makes libxc 4.4 mandatory for Xen support. > > Patch 2-3 remove uses of /proc/iomem in favour of libxc. > > Patch 4 adds the support for loading, unloading, and exec'ing an image > in Xen. > > This series explicitly drops support for older version of libxc/Xen as > supporting kexec on these hypervisors requires kernel support that > will never be available upstream. > > Changes in v7: > - extend extra segment to 0-1 MiB so the VGA memory is accesible by > the image. > > Changes in v6: > - Close libxc handle a few error paths. > > Changes in v5: > - Dropped the purgatory patch. > > Changes in v4: > - KEXEC_ARCH_DEFAULT => EM_386 > - add segment in the crash case for the backup src. > > Changes in v3 (not posted): > - Rebased > > Changes in v2: > - Patch from Don Sultz: use libxc for vmcoreinfo. > - Use xc_get_max_cpus() to get number of PCPUs. > - Enable unload and exec of images. For whole Xen kexec/kdump kexec-tools series: Reviewed-by: Daniel Kiper Tested-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv10 0/9] Xen: extend kexec hypercall for use with pv-ops kernels
On Thu, Nov 07, 2013 at 10:16:51PM +0100, Daniel Kiper wrote: > On Wed, Nov 06, 2013 at 02:49:37PM +, David Vrabel wrote: > > The series (for Xen 4.4) improves the kexec hypercall by making Xen > > responsible for loading and relocating the image. This allows kexec > > to be usable by pv-ops kernels and should allow kexec to be usable > > from a HVM or PVH privileged domain. > > > > I have now tested this with a Linux kernel image using the VGA console > > which was what was causing problems in v9 (this turned out to be a > > kexec-tools bug). > > > > The required patch series for kexec-tools will be posted shortly and > > are available from the xen-v7 branch of: > > In general it works. However, quite often I am not able to execute panic > kernel. Machine hangs with following message: > > (XEN) Domain 0 crashed: Executing crash image > > gdb shows: > > (gdb) bt > #0 0x82d0801a0092 in do_nmi_crash (regs=) at crash.c:113 > #1 0x82d0802281d9 in nmi_crash () at entry.S:666 > #2 0x in ?? () > (gdb) > > Especially second bt line scares me... ;-))) > > I have not been able to identify why NMI was activated because > stack is completely cleared. I tried to record execution in gdb > but it stops with following message: > > cpumask_clear_cpu (dstp=0x82d0802f7f78 , cpu=0) > at /srv/dev/xen/xen_20130413_20131107.kexec/xen/include/xen/cpumask.h:108 > 108 clear_bit(cpumask_check(cpu), dstp->bits); > Process record: failed to record execution log. > > Do you know how to find out why NMI was activated? > > I am able almost always reproduce this issue doing this: > - boot Xen, > - load panic kernel, > - echo c > /proc/sysrq-trigger, > - reboot from command line, > - boot Xen, > - load panic kernel, > - echo c > /proc/sysrq-trigger. I am not able to reproduce this on real hardware. Sorry for confusion. Hence, for whole Xen kexec/kdump series: Reviewed-by: Daniel Kiper Tested-by: Daniel Kiper Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Xen-devel] [PATCHv10 0/9] Xen: extend kexec hypercall for use with pv-ops kernels
On Fri, Nov 08, 2013 at 10:42:51AM -0500, Konrad Rzeszutek Wilk wrote: > On Fri, Nov 08, 2013 at 07:15:00AM -0800, Daniel Kiper wrote: > > On Fri, Nov 08, 2013 at 02:01:28PM +, Andrew Cooper wrote: > > > On 08/11/13 13:19, Jan Beulich wrote: > > > >>>> On 08.11.13 at 14:13, David Vrabel wrote: > > > >> Keir, > > > >> > > > >> Sorry, forgot to CC you on this series. > > > >> > > > >> Can we have your opinion on whether this kexec series can be merged? > > > >> And if not, what further work and/or testing is required? > > > > Just to clarify - unless I missed something, there was still no > > > > review of this from Daniel or someone else known to be > > > > familiar with the subject. If Keir gave his ack, formally this > > > > could go in, but I wouldn't feel too well with that (the more > > > > that apart from not having reviewed it, Daniel seems to also > > > > continue to have problems with it). > > > > > > > > Jan > > > > > > Can I have myself deemed to be familiar with the subject as far as this > > > is concerned? > > > > > > A noticeable quantity of my contributions to Xen have been in the kexec > > > / crash areas, and I am the author of the xen-crashdump-analyser. > > > > > > I do realise that I certainly not impartial as far as this series is > > > concerned, being a co-developer. > > > > > > Davids statement of "the current implementation is so broken[1] and > > > useless[2] that..." is completely accurate. It is frankly a miracle > > > that the current code ever worked at all (and from XenServers point of > > > view, failed far more often than it worked). > > > > > > > > > For reference, XenServer 6.2 shipped with approximately v7 of this > > > series, and an appropriate kexec-tools and xen-crashdump-analyser. > > > Since we put the code in, we have not had a single failure-to-kexec in > > > automated testing (both specific crash tests, and from unexpected host > > > crashes), whereas we were seeing reliable failures to crash on most of > > > our test infrastructure. > > > > > > In stark contrast to previous versions of XenServer, we have not had a > > > single customer reported host crash where the kexec path has failed. > > > There was one systematic failure where the HPSA driver was unhappy with > > > the state of the hardware, resulting in no root filesystem to write logs > > > to, and a repeated panic and Xen deadlock in the queued invalidation > > > codepath. > > > > Andrew, if it runs on all your hardware it does not mean that it runs > > everywhere. I have discovered the problem (I hope the last one) and it > > should be taken into consideration. Another question is what is the > > source of this problem. Maybe QEMU but it should be checked and not > > ignored. > > I think the question is that the feature freeze is the 18th - and whether > this single bug should halt the integration of this whole patchset. > > Or that it is OK to put in the patchset in and deal with the bugs > and not stall this initial patchset. I have never stated that I would like to block this patch series indefinitely due to this one bug (I am still not sure that this is a bug; Currently, I feel that I am only one person who tries to verify that). We have more then one week and I think that we are able to discover what is going on. If not I think that we can workout reasonable solution for this issue (as we did in other cases). Last but not least, I would like to underline that I wish that this patch series were included in Xen 4.4 too. However, it must be done in sensible way. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [Xen-devel] [PATCHv10 0/9] Xen: extend kexec hypercall for use with pv-ops kernels
On Fri, Nov 08, 2013 at 02:01:28PM +, Andrew Cooper wrote: > On 08/11/13 13:19, Jan Beulich wrote: > On 08.11.13 at 14:13, David Vrabel wrote: > >> Keir, > >> > >> Sorry, forgot to CC you on this series. > >> > >> Can we have your opinion on whether this kexec series can be merged? > >> And if not, what further work and/or testing is required? > > Just to clarify - unless I missed something, there was still no > > review of this from Daniel or someone else known to be > > familiar with the subject. If Keir gave his ack, formally this > > could go in, but I wouldn't feel too well with that (the more > > that apart from not having reviewed it, Daniel seems to also > > continue to have problems with it). > > > > Jan > > Can I have myself deemed to be familiar with the subject as far as this > is concerned? > > A noticeable quantity of my contributions to Xen have been in the kexec > / crash areas, and I am the author of the xen-crashdump-analyser. > > I do realise that I certainly not impartial as far as this series is > concerned, being a co-developer. > > Davids statement of "the current implementation is so broken[1] and > useless[2] that..." is completely accurate. It is frankly a miracle > that the current code ever worked at all (and from XenServers point of > view, failed far more often than it worked). > > > For reference, XenServer 6.2 shipped with approximately v7 of this > series, and an appropriate kexec-tools and xen-crashdump-analyser. > Since we put the code in, we have not had a single failure-to-kexec in > automated testing (both specific crash tests, and from unexpected host > crashes), whereas we were seeing reliable failures to crash on most of > our test infrastructure. > > In stark contrast to previous versions of XenServer, we have not had a > single customer reported host crash where the kexec path has failed. > There was one systematic failure where the HPSA driver was unhappy with > the state of the hardware, resulting in no root filesystem to write logs > to, and a repeated panic and Xen deadlock in the queued invalidation > codepath. Andrew, if it runs on all your hardware it does not mean that it runs everywhere. I have discovered the problem (I hope the last one) and it should be taken into consideration. Another question is what is the source of this problem. Maybe QEMU but it should be checked and not ignored. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv10 0/9] Xen: extend kexec hypercall for use with pv-ops kernels
On Fri, Nov 08, 2013 at 01:13:59PM +, David Vrabel wrote: [...] > > (XEN) Domain 0 crashed: Executing crash image > > > > gdb shows: > > > > (gdb) bt > > #0 0x82d0801a0092 in do_nmi_crash (regs=) at crash.c:113 > > #1 0x82d0802281d9 in nmi_crash () at entry.S:666 > > #2 0x in ?? () > > (gdb) > > > > Especially second bt line scares me... ;-))) > > > > I have not been able to identify why NMI was activated because > > stack is completely cleared. > > All this you have described here is correct and expected behavior, > which, quite frankly, you should have been able to see with even the > most cursory look at the code. This is more a fun stuff than a real concern. That is why I have added smile at the end of my statement. nmi_crash () at entry.S:666 is a quite interesting coincidence for me... ;-))) Anyway, it is interesting why all CPUs were stopped at this stage. One should execute kdump code still. I will try to reproduce this on real hardware. Daniel ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
