From: Denis Mukhin <dmkhn@proton.me>
To: Jan Beulich <jbeulich@suse.com>
Cc: andrew.cooper3@citrix.com, anthony.perard@vates.tech,
julien@xen.org, michal.orzel@amd.com, roger.pau@citrix.com,
sstabellini@kernel.org, dmukhin@ford.com,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v3] xen/console: print Xen version via keyhandler
Date: Fri, 14 Feb 2025 20:07:09 +0000 [thread overview]
Message-ID: <tzAUNSv_X55I4CsPnmpDvLQ09p6mK5og088aJunH0vLh1CcZ3K1WYZpRGwCVbigzsiyp8MJPSrObT6rX1fqri2d3-PToFgXSMVsbWi5PHN0=@proton.me> (raw)
In-Reply-To: <e80d6139-b918-4830-9db9-aac297446e7e@suse.com>
On Thursday, February 13th, 2025 at 11:54 PM, Jan Beulich <jbeulich@suse.com> wrote:
>
>
> On 13.02.2025 22:41, dmkhn@proton.me wrote:
>
> > Add Xen version printout to 'h' keyhandler output.
> >
> > That is useful for debugging systems that have been left intact for a long
> > time.
> >
> > Signed-off-by: Denis Mukhin dmukhin@ford.com
> > ---
> > Changes since v2:
> > - moved new function declarations to xen/lib.h
> > - dropped xen_ prefix in new functions
> > ---
> > xen/common/keyhandler.c | 4 ++++
> > xen/common/version.c | 20 ++++++++++++++++++--
> > xen/drivers/char/console.c | 8 +++-----
> > xen/include/xen/lib.h | 3 +++
> > 4 files changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
> > index 6ea54838d4..0bb842ec00 100644
> > --- a/xen/common/keyhandler.c
> > +++ b/xen/common/keyhandler.c
> > @@ -129,6 +129,10 @@ static void cf_check show_handlers(unsigned char key)
> > unsigned int i;
> >
> > printk("'%c' pressed -> showing installed handlers\n", key);
> > +
> > + print_version();
> > + print_build_id();
> > +
> > for ( i = 0; i < ARRAY_SIZE(key_table); i++ )
> > if ( key_table[i].fn )
> > printk(" key '%c' (ascii '%02x') => %s\n",
> > diff --git a/xen/common/version.c b/xen/common/version.c
> > index bc3714b45f..8672d5544e 100644
> > --- a/xen/common/version.c
> > +++ b/xen/common/version.c
> > @@ -210,9 +210,25 @@ void __init xen_build_init(void)
> > }
> > }
> > #endif /* CONFIG_X86 */
> > - if ( !rc )
> > - printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);
> > }
> > +
> > +void print_version(void)
> > +{
> > + printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n",
> > + xen_major_version(), xen_minor_version(), xen_extra_version(),
> > + xen_compile_by(), xen_compile_domain(), xen_compiler(),
> > + xen_build_info(), xen_compile_date());
> > +
> > + printk("Latest ChangeSet: %s\n", xen_changeset());
> > +}
> > +
> > +void print_build_id(void)
> > +{
> > + BUG_ON(!build_id_p);
> > + BUG_ON(!build_id_len);
>
>
> Technically one of the two would likely suffice, if we need such checks
> at all. Question is why you added them. After all ...
I added assertions so that any misuse can bee easily seen.
But I did not know about XEN_HAS_BUILD_ID switch.
>
> > + printk(XENLOG_INFO "build-id: %*phN\n", build_id_len, build_id_p);
>
>
> ... this isn't supposed to malfunction when passed a NULL pointer (with
> zero length). If it can malfunction, it wants fixing there imo. And that
> extends to the case of non-zero length as well: Any extensions to %p
> that we add would better retain the basic property of %p printing when
> NULL is passed as argument. (I'm sorry for not paying enough attention
> to this on v2 already.)
No problem! Thanks for the feedback, much appreciated.
>
> (Later) Oh, actually these BUG_ON() are both wrong. They would trigger
> when building with a build-id-incapable linker. See the detection logic
> in the top level Makefile (search for XEN_HAS_BUILD_ID). To retain
> original behavior you will want to make the printk() conditional upon
> e.g. build_id_len being non-zero.
Oh, I see.
%p in printk() behaves correctly when passing NULL pointer - it prints
nothing.
>
> Jan
prev parent reply other threads:[~2025-02-14 20:07 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-13 21:41 [PATCH v3] xen/console: print Xen version via keyhandler dmkhn
2025-02-14 7:54 ` Jan Beulich
2025-02-14 20:07 ` Denis Mukhin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='tzAUNSv_X55I4CsPnmpDvLQ09p6mK5og088aJunH0vLh1CcZ3K1WYZpRGwCVbigzsiyp8MJPSrObT6rX1fqri2d3-PToFgXSMVsbWi5PHN0=@proton.me' \
--to=dmkhn@proton.me \
--cc=andrew.cooper3@citrix.com \
--cc=anthony.perard@vates.tech \
--cc=dmukhin@ford.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.