From: Oleksii <oleksii.kurochko@gmail.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Gianluca Guida <gianluca@rivosinc.com>,
George Dunlap <george.dunlap@citrix.com>, Wei Liu <wl@xen.org>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v5 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
Date: Tue, 07 Mar 2023 15:31:59 +0200 [thread overview]
Message-ID: <3d6d2a4f89744e8b6814e668244fe8058df04d64.camel@gmail.com> (raw)
In-Reply-To: <7be952d9-efea-23ec-0c5f-cfce03bee6a0@suse.com>
On Tue, 2023-03-07 at 14:16 +0100, Jan Beulich wrote:
> On 07.03.2023 14:13, Oleksii wrote:
> > > > > > > +
> > > > > > > +#define BUG_FRAME(type, line, ptr, second_frame, msg) do
> > > > > > > { \
> > > > > > > + BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH +
> > > > > > > BUG_LINE_HI_WIDTH)); \
> > > > > > > + BUILD_BUG_ON((type) >=
> > > > > > > BUGFRAME_NR); \
> > > > > > > + asm volatile (
> > > > > > > _ASM_BUGFRAME_TEXT(second_frame)
> > > > > > > \
> > > > > > > + :: _ASM_BUGFRAME_INFO(type, line,
> > > > > > > ptr,
> > > > > > > msg)
> > > > > > > ); \
> > > > > > > +} while (0)
> > > > >
> > > > > Isn't this tied to BUG_FRAME_STRUCT being defined (or not)?
> > > > > At
> > > > > least
> > > > > the 1st BUILD_BUG_ON() looks problematic if an arch wasn't to
> > > > > use
> > > > > the generic struct: With how you have things right now
> > > > > BUG_LINE_{LO,HI}_WIDTH may not be defined, and the check
> > > > > would
> > > > > also
> > > > > be at risk of causing false positives. Perhaps it's
> > > > > appropriate
> > > > > to
> > > > > have a separate #ifdef (incl the distinct identifier used),
> > > > > but
> > > > > that
> > > > > first BUILD_BUG_ON() needs abstracting out.
> > > Missed that. Thanks.
> > > I'll introduce that a separate #ifdef before BUG_FRAME:
> > >
> > > #ifndef BUILD_BUG_ON_LINE_WIDTH
> > > #define BUILD_BUG_ON_LINE_WIDTH \
> > > BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH +
> > > BUG_LINE_HI_WIDTH))
> > > #endif
> > I think even better will be to do in the following way:
> >
> > #ifndef LINE_WIDTH
> > #define LINE_WIDTH (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)
> > #endif
> >
> > #define BUG_FRAME(type, line, ptr, second_frame, msg) do
> > {
> > \
> > BUILD_BUG_ON((line) >>
> > LINE_WIDTH);
> > \
> > BUILD_BUG_ON((type) >=
> > BUGFRAME_NR);
> > \
> > asm volatile (
> > _ASM_BUGFRAME_TEXT(second_frame)
> > \
> > :: _ASM_BUGFRAME_INFO(type, line, ptr, msg)
> > );
> > \
> > } while (false)
>
> Not sure - that'll still require arches to define LINE_WIDTH. What
> if they store the line number in a 32-bit field? Then defining
> LINE_WIDTH to 32 would yield undefined behavior here.
>
It might be an issue.
Than it will be better to have function-like macros mentioned in
previous e-mail.
Thanks
~ Oleksii
next prev parent reply other threads:[~2023-03-07 13:32 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-03 10:38 [PATCH v5 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-03-03 10:38 ` [PATCH v5 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-03-06 10:17 ` Jan Beulich
2023-03-07 11:32 ` Oleksii
2023-03-07 12:54 ` Jan Beulich
2023-03-07 13:13 ` Oleksii
2023-03-07 13:16 ` Jan Beulich
2023-03-07 13:31 ` Oleksii [this message]
2023-03-06 10:41 ` Jan Beulich
2023-03-07 11:34 ` Oleksii
2023-03-03 10:38 ` [PATCH v5 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
2023-03-06 10:24 ` Jan Beulich
2023-03-03 10:38 ` [PATCH v5 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
2023-03-03 11:29 ` Jan Beulich
2023-03-03 11:55 ` Oleksii
2023-03-03 10:38 ` [PATCH v5 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
2023-03-06 10:36 ` Jan Beulich
2023-03-07 12:52 ` Oleksii
2023-03-07 12:59 ` Jan Beulich
2023-03-07 13:37 ` Oleksii
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=3d6d2a4f89744e8b6814e668244fe8058df04d64.camel@gmail.com \
--to=oleksii.kurochko@gmail.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=gianluca@rivosinc.com \
--cc=jbeulich@suse.com \
--cc=julien@xen.org \
--cc=sstabellini@kernel.org \
--cc=wl@xen.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.