On 29. Apr 2019, at 17:47, Ross Lagerwall <ross.lagerw...@citrix.com<mailto:ross.lagerw...@citrix.com>> wrote:
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 <wipa...@amazon.de<mailto:wipa...@amazon.de>> Reviewed-by: Andra-Irina Paraschiv <andra...@amazon.com<mailto:andra...@amazon.com>> Reviewed-by: Bjoern Doebel <doe...@amazon.de<mailto:doe...@amazon.de>> Reviewed-by: Norbert Manthey <nmant...@amazon.de<mailto:nmant...@amazon.de>> --- 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. Oh, certainly. I forgot to update the commits involved properly. ACK. Will fix. 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(). ACK. Will fix. + 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 That’s true. The .altinstr_replacement group_size should be 0 (aka undefined). ACK. Will fix. 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. No this was not the goal. I am explaining it below. 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. As discussed when reviewing [1], I need a function telling which section is special. By special I mean: a) should not be a subject of extra validation (See [2]) b) should not be a subject of STN_UNDEF purging (See [3]) Thus, I want to explicitly call out all "special" sections by their name and specify their group_size whenever applicable (I did it incorrectly for .altinstr_replacement, as you have spotted - thanks!) in the common array and use it with the helper function or for obtaining a group_size. [1] [livepatch-build-tools part2 3/6] create-diff-object: Add is_special_section() helper function [2] [livepatch-build-tools part3 2/3] create-diff-object: Extend patchability verification: STN_UNDEF [3] [livepatch-build-tools part3 3/3] create-diff-object: Strip all undefined entires of known size Regards, -- Ross Lagerwall Best Regards, Pawel Wieczorkiewicz Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
