Re: [PATCH 2/4] xen/virtual-region: Rework how bugframe linkage works

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.03.2024 12:04, Andrew Cooper wrote:
> The start/stop1/etc linkage scheme predates struct virtual_region, and as
> setup_virtual_regions() shows, it's awkward to express in the new scheme.
> 
> Change the linker to provide explicit start/stop symbols for each bugframe
> type, and change virtual_region to have a stop pointer rather than a count.
> 
> This marginly simplifies both do_bug_frame()s and prepare_payload(), but it
> massively simplifies setup_virtual_regions() by allowing the compiler to
> initialise the .frame[] array at build time.
> 
> virtual_region.c is the only user of the linker symbols, and this is unlikely
> to change given the purpose of struct virtual_region, so move their externs
> out of bug.h
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Of course ...

> --- a/xen/common/virtual_region.c
> +++ b/xen/common/virtual_region.c
> @@ -9,12 +9,25 @@
>  #include <xen/spinlock.h>
>  #include <xen/virtual_region.h>
>  
> +extern const struct bug_frame
> +    __start_bug_frames_0[], __stop_bug_frames_0[],
> +    __start_bug_frames_1[], __stop_bug_frames_1[],
> +    __start_bug_frames_2[], __stop_bug_frames_2[],
> +    __start_bug_frames_3[], __stop_bug_frames_3[];
> +
>  static struct virtual_region core = {
>      .list = LIST_HEAD_INIT(core.list),
>      .text_start = _stext,
>      .text_end = _etext,
>      .rodata_start = _srodata,
>      .rodata_end = _erodata,
> +
> +    .frame = {
> +        { __start_bug_frames_0, __stop_bug_frames_0 },
> +        { __start_bug_frames_1, __stop_bug_frames_1 },
> +        { __start_bug_frames_2, __stop_bug_frames_2 },
> +        { __start_bug_frames_3, __stop_bug_frames_3 },
> +    },
>  };
>  
>  /* Becomes irrelevant when __init sections are cleared. */
> @@ -22,6 +35,13 @@ static struct virtual_region core_init __initdata = {
>      .list = LIST_HEAD_INIT(core_init.list),
>      .text_start = _sinittext,
>      .text_end = _einittext,
> +
> +    .frame = {
> +        { __start_bug_frames_0, __stop_bug_frames_0 },
> +        { __start_bug_frames_1, __stop_bug_frames_1 },
> +        { __start_bug_frames_2, __stop_bug_frames_2 },
> +        { __start_bug_frames_3, __stop_bug_frames_3 },
> +    },
>  };

... this is now calling yet louder for splitting runtime from init bug
frame records.

Jan