Re: [PATCH v2 3/4] xen/virtual-region: Link the list build time

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Andrew,

On 10/04/2024 20:42, Andrew Cooper wrote:
> 
> 
> Given 3 statically initialised objects, its easy to link the list at build
> time.  There's no need to do it during runtime at boot (and with IRQs-off,
> even).
> 
> As a consequence, register_virtual_region() can now move inside ifdef
> CONFIG_LIVEPATCH like unregister_virtual_region().
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Maybe with ...
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> CC: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> v2:
>  * Collect the initialisers togoether too.
> ---
>  xen/common/virtual_region.c | 37 ++++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
> index 7d8bdeb61282..db3e0dc9fe74 100644
> --- a/xen/common/virtual_region.c
> +++ b/xen/common/virtual_region.c
> @@ -15,8 +15,18 @@ extern const struct bug_frame
>      __start_bug_frames_2[], __stop_bug_frames_2[],
>      __start_bug_frames_3[], __stop_bug_frames_3[];
> 
> +/*
> + * For the built-in regions, the double linked list can be constructed at
> + * build time.  Forward-declare the elements and their initialisers.
> + */
> +static struct list_head virtual_region_list;
> +static struct virtual_region core, core_init;
... empty line here for better readability

> +#define LIST_ENTRY_HEAD() { .next = &core.list,           .prev = 
> &core_init.list }
> +#define LIST_ENTRY_CORE() { .next = &core_init.list,      .prev = 
> &virtual_region_list }
> +#define LIST_ENTRY_INIT() { .next = &virtual_region_list, .prev = &core.list 
> }

~Michal