From: Jan Beulich <jbeulich@suse.com>
To: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Cc: "Julien Grall" <julien@xen.org>,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Gianluca Guida" <gianluca@rivosinc.com>,
"Bertrand Marquis" <bertrand.marquis@arm.com>,
"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
"George Dunlap" <george.dunlap@citrix.com>,
"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v5 2/4] xen: change <asm/bug.h> to <xen/bug.h>
Date: Mon, 6 Mar 2023 11:24:06 +0100 [thread overview]
Message-ID: <5c7e4584-9c99-1ecd-e231-438b89c11805@suse.com> (raw)
In-Reply-To: <f108147036c9e35b156b45c0f52779cea09025c9.1677839409.git.oleksii.kurochko@gmail.com>
On 03.03.2023 11:38, Oleksii Kurochko wrote:
> The idea of the patch is to change all <asm/bug.h> to <xen/bug.h> and
> keep Xen compilable with adding only minimal amount of changes:
> 1. It was added "#include <xen/types.h>" to ARM's "<asm/bug.h>" as it
> uses uint_{16,32}t in 'struct bug_frame'.
> 2. It was added '#define BUG_FRAME_STRUCT' which means that ARM hasn't
> been switched to generic implementation yet.
> 3. It was added '#define BUG_FRAME_STRUCT' which means that x86 hasn't
> been switched to generic implementation yet.
> 4. BUGFRAME_* and _start_bug_frame[], _stop_bug_frame_*[] were removed
> for ARM & x86 to deal with compilation errors such as:
> redundant redeclaration of ...
>
> In the following two patches x86 and ARM archictectures will be
> switched fully:
> * xen/arm: switch ARM to use generic implementation of bug.h
> * xen/x86: switch x86 to use generic implemetation of bug.h
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V5:
> - Nothing changed
> ---
> Changes in V4:
> - defines BUG_DISP_WIDTH, BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH were moved into
> "ifndef BUG_FRAME_STRUCT" in <xen/bug.h> as they are specific for 'struct bug_frame' and so should
> co-exist together. So the defines were back to <asm/bug.h> until BUG_FRAME_STRUCT will be defined in
> <asm/bug.h>.
> - Update the comment message.
> ---
> Changes in V3:
> * Update patch 2 not to break compilation: move some parts from patches 3 and 4
> to patch 2:
> * move some generic parts from <asm/bug.h> to <xen/bug.h>
> * add define BUG_FRAME_STRUCT in ARM's <asm/bug.h>
> ---
> Changes in V2:
> * Put [PATCH v1 4/4] xen: change <asm/bug.h> to <xen/bug.h> as second patch,
> update the patch to change all <asm/bug.h> to <xen/bug.h> among the whole project
> to not break build.
> * Update the commit message.
> ---
> xen/arch/arm/include/asm/bug.h | 17 ++++-------------
> xen/arch/arm/include/asm/div64.h | 2 +-
> xen/arch/arm/vgic/vgic-v2.c | 2 +-
> xen/arch/arm/vgic/vgic.c | 2 +-
> xen/arch/x86/acpi/cpufreq/cpufreq.c | 2 +-
> xen/arch/x86/include/asm/asm_defns.h | 2 +-
> xen/arch/x86/include/asm/bug.h | 15 ++-------------
> xen/drivers/cpufreq/cpufreq.c | 2 +-
> xen/include/xen/lib.h | 2 +-
> 9 files changed, 13 insertions(+), 33 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
> index f4088d0913..9315662c6e 100644
> --- a/xen/arch/arm/include/asm/bug.h
> +++ b/xen/arch/arm/include/asm/bug.h
> @@ -1,6 +1,8 @@
> #ifndef __ARM_BUG_H__
> #define __ARM_BUG_H__
>
> +#include <xen/types.h>
> +
> #if defined(CONFIG_ARM_32)
> # include <asm/arm32/bug.h>
> #elif defined(CONFIG_ARM_64)
> @@ -13,6 +15,8 @@
> #define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
> #define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
>
> +#define BUG_FRAME_STRUCT
> +
> struct bug_frame {
> signed int loc_disp; /* Relative address to the bug address */
> signed int file_disp; /* Relative address to the filename */
> @@ -26,13 +30,6 @@ struct bug_frame {
> #define bug_line(b) ((b)->line)
> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>
> -#define BUGFRAME_run_fn 0
> -#define BUGFRAME_warn 1
> -#define BUGFRAME_bug 2
> -#define BUGFRAME_assert 3
> -
> -#define BUGFRAME_NR 4
> -
> /* Many versions of GCC doesn't support the asm %c parameter which would
> * be preferable to this unpleasantness. We use mergeable string
> * sections to avoid multiple copies of the string appearing in the
> @@ -89,12 +86,6 @@ struct bug_frame {
> unreachable(); \
> } while (0)
>
> -extern const struct bug_frame __start_bug_frames[],
> - __stop_bug_frames_0[],
> - __stop_bug_frames_1[],
> - __stop_bug_frames_2[],
> - __stop_bug_frames_3[];
> -
> #endif /* __ARM_BUG_H__ */
> /*
> * Local variables:
> diff --git a/xen/arch/arm/include/asm/div64.h b/xen/arch/arm/include/asm/div64.h
> index 1cd58bc51a..fc667a80f9 100644
> --- a/xen/arch/arm/include/asm/div64.h
> +++ b/xen/arch/arm/include/asm/div64.h
> @@ -74,7 +74,7 @@
>
> #elif __GNUC__ >= 4
>
> -#include <asm/bug.h>
> +#include <xen/bug.h>
>
> /*
> * If the divisor happens to be constant, we determine the appropriate
> diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
> index 1a99d3a8b4..c90e88fddb 100644
> --- a/xen/arch/arm/vgic/vgic-v2.c
> +++ b/xen/arch/arm/vgic/vgic-v2.c
> @@ -16,8 +16,8 @@
> */
>
> #include <asm/new_vgic.h>
> -#include <asm/bug.h>
> #include <asm/gic.h>
> +#include <xen/bug.h>
> #include <xen/sched.h>
> #include <xen/sizes.h>
>
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index f0f2ea5021..b9463a5f27 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -15,9 +15,9 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <xen/bug.h>
> #include <xen/list_sort.h>
> #include <xen/sched.h>
> -#include <asm/bug.h>
> #include <asm/event.h>
> #include <asm/new_vgic.h>
>
> diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> index c27cbb2304..18ff2a443b 100644
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -27,6 +27,7 @@
> * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> */
>
> +#include <xen/bug.h>
> #include <xen/types.h>
> #include <xen/errno.h>
> #include <xen/delay.h>
> @@ -35,7 +36,6 @@
> #include <xen/sched.h>
> #include <xen/timer.h>
> #include <xen/xmalloc.h>
> -#include <asm/bug.h>
> #include <asm/msr.h>
> #include <asm/io.h>
> #include <asm/processor.h>
> diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
> index d9431180cf..a8526cf36c 100644
> --- a/xen/arch/x86/include/asm/asm_defns.h
> +++ b/xen/arch/x86/include/asm/asm_defns.h
> @@ -6,7 +6,7 @@
> /* NB. Auto-generated from arch/.../asm-offsets.c */
> #include <asm/asm-offsets.h>
> #endif
> -#include <asm/bug.h>
> +#include <xen/bug.h>
> #include <asm/x86-defns.h>
> #include <xen/stringify.h>
> #include <asm/cpufeature.h>
While there's an unhelpful mix of asm/ and xen/ here already, may I ask
that you try to avoid making things yet worse: Unless there's a reason
not to, please move the added line past asm/x86-defns.h, adjacent to
xen/stringify.h. Then non-Arm parts
Acked-by: Jan Beulich <jbeulich@suse.com>
(assuming of course no further large rework is necessary because of the
comments on patch 1).
Jan
next prev parent reply other threads:[~2023-03-06 10:24 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
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 [this message]
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=5c7e4584-9c99-1ecd-e231-438b89c11805@suse.com \
--to=jbeulich@suse.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=andrew.cooper3@citrix.com \
--cc=bertrand.marquis@arm.com \
--cc=george.dunlap@citrix.com \
--cc=gianluca@rivosinc.com \
--cc=julien@xen.org \
--cc=oleksii.kurochko@gmail.com \
--cc=roger.pau@citrix.com \
--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.