On 18/03/2024 1:17 pm, Jan Beulich wrote:
> 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.coop...@citrix.com>
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
>
> 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.
How do you propose we do this?
We can probably do it for *.init.o objects by renaming the bugframe
sections too, but unless we can do it for *every* bugframe emitted in
all init code, splitting this will break things.
~Andrew
