Re: [Xen-devel] [livepatch-build-tools part2 5/6] create-diff-object: Add new entries to special sections array

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 4/16/19 1:07 PM, Pawel Wieczorkiewicz wrote:
> Handle .livepatch.hooks* and .altinstr_replacement sections as the
> special sections with assigned group_size resolution function.
> By default each .livepatch.hooks* sections' entry is 8 bytes long (a
> pointer). The .altinstr_replacement section follows the .altinstructions
> section settings.
>
> Allow to specify different .livepatch.hooks* section entry size using
> shell environment variable HOOK_STRUCT_SIZE.
>
> Cleanup an incorrect indentation around group_size resulution functions.
>
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
> Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>
> Reviewed-by: Norbert Manthey <nmanthey@xxxxxxxxx>
> ---
>   create-diff-object.c | 87 ++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 54 insertions(+), 33 deletions(-)
>
> diff --git a/create-diff-object.c b/create-diff-object.c
> index b0b4dcb..f6060cd 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -960,51 +960,64 @@ static void kpatch_mark_constant_labels_same(struct 
> kpatch_elf *kelf)
Fix the indentation in the patch which introduces the problem rather 
than fixing it in a subsequent patch.
>   
>   static int bug_frames_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("BUG_STRUCT_SIZE");
> -        size = str ? atoi(str) : 8;
> -    }
> -
> -    return size;
> +       static int size = 0;
> +       char *str;
> +       if (!size) {
> +               str = getenv("BUG_STRUCT_SIZE");
> +               size = str ? atoi(str) : 8;
> +       }
> +
> +       return size;
>   }
>   
>   static int bug_frames_3_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("BUG_STRUCT_SIZE");
> -        size = str ? atoi(str) : 16;
> -    }
> -
> -    return size;
> +       static int size = 0;
> +       char *str;
> +       if (!size) {
> +               str = getenv("BUG_STRUCT_SIZE");
> +               size = str ? atoi(str) : 16;
> +       }
> +
> +       return size;
>   }
>   
>   static int ex_table_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("EX_STRUCT_SIZE");
> -        size = str ? atoi(str) : 8;
> -    }
> -
> -    return size;
> +       static int size = 0;
> +       char *str;
> +       if (!size) {
> +               str = getenv("EX_STRUCT_SIZE");
> +               size = str ? atoi(str) : 8;
> +       }
> +
> +       return size;
>   }
>   
>   static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
>   {
> -    static int size = 0;
> -    char *str;
> -    if (!size) {
> -        str = getenv("ALT_STRUCT_SIZE");
> -        size = str ? atoi(str) : 12;
> -    }
> -
> -    printf("altinstr_size=%d\n", size);
> -    return size;
> +       static int size = 0;
> +       char *str;
> +       if (!size) {
> +               str = getenv("ALT_STRUCT_SIZE");
> +               size = str ? atoi(str) : 12;
> +       }
> +
> +       printf("altinstr_size=%d\n", size);
> +       return size;
> +}
> +
> +static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
> +{
> +       static int size = 0;
> +       char *str;
> +       if (!size) {
> +               str = getenv("HOOK_STRUCT_SIZE");
> +               size = str ? atoi(str) : 8;
> +       }
> +
> +       printf("livepatch_hooks_size=%d\n", size);
If you want to keep this debugging output, rather use log_debug().

> +       return size;
>   }
>   
>   /*
> @@ -1084,6 +1097,14 @@ static struct special_section special_sections[] = {
>                 .name           = ".altinstructions",
>                 .group_size     = altinstructions_group_size,
>         },
> +       {
> +               .name           = ".altinstr_replacement",
> +               .group_size     = altinstructions_group_size,
> +       },
> +       {
> +               .name           = ".livepatch.hooks",
> +               .group_size     = livepatch_hooks_group_size,
> +       },
>         {},
>   };
>   
I don't understand the point of this change.

IIUC unlike .altinstructions, the .altinstr_replacement section does not 
have a fixed group size, so this change would not work other than by 
luck. If the goal was to prune bits of the .altinstr_replacement table 
that do not need to be included, you need to update the code in 
kpatch_process_special_sections() which has special handling for 
.altinstr_replacement. It needs to parse the updated, pruned 
.altinstructions section and regenerate .altinstr_replacement, including 
only what is needed.
I don't know why .livepatch.hooks is included in this list either since 
all hooks are always included and there is nothing to regenerate/prune.
Regards,
--
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel