| Message ID | 20170726194756.20265-5-konrad@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> Konrad Rzeszutek Wilk <konrad@kernel.org> 07/26/17 9:50 PM >>> >On x86 the bloat-o-meter detects that with this change the file shrinks: >add/remove: 1/0 grow/shrink: 0/2 up/down: 156/-367 (-211) >function old new delta >get_page_from_gfn - 156 +156 >do_mmu_update 4578 4569 -9 >do_mmuext_op 5604 5246 -358 >Total: Before=3170439, After=3170228, chg -0.01% This looks unexpected, and hence I'd like to ask for an explanation. If anything I'd expect the image size to grow (slightly). >--- a/xen/include/asm-x86/alternative.h >+++ b/xen/include/asm-x86/alternative.h >@@ -56,10 +56,12 @@ extern void alternative_instructions(void); > >#define ALTERNATIVE_N(newinstr, feature, number) \ >".pushsection .altinstructions,\"a\"\n" \ >+ ".p2align 2\n" \ Can't this then be accompanied by dropping the (over-)alignment done in xen.lds.S? >ALTINSTR_ENTRY(feature, number) \ >".section .discard,\"a\",@progbits\n" \ >DISCARD_ENTRY(number) \ >".section .altinstr_replacement, \"ax\"\n" \ >+ ".p2align 2\n" \ This surely isn't needed on x86. Jan
diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h index 6cc9d0dc5f..cd1373fdd5 100644 --- a/xen/include/asm-arm/alternative.h +++ b/xen/include/asm-arm/alternative.h @@ -54,9 +54,11 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en oldinstr "\n" \ "662:\n" \ ".pushsection .altinstructions,\"a\"\n" \ + ".p2align 2\n" \ ALTINSTR_ENTRY(feature) \ ".popsection\n" \ ".pushsection .altinstr_replacement, \"a\"\n" \ + ".p2align 2\n" \ "663:\n\t" \ newinstr "\n" \ "664:\n\t" \ @@ -84,6 +86,7 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en .if \enable 661: \insn1 662: .pushsection .altinstructions, "a" + .p2align 2 altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f .popsection .pushsection .altinstr_replacement, "ax" @@ -103,6 +106,7 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en .macro alternative_if_not cap, enable = 1 .if \enable .pushsection .altinstructions, "a" + .p2align 2 altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f .popsection 661: diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h index db4f08e0e7..e667ccfbdb 100644 --- a/xen/include/asm-x86/alternative.h +++ b/xen/include/asm-x86/alternative.h @@ -56,10 +56,12 @@ extern void alternative_instructions(void); #define ALTERNATIVE_N(newinstr, feature, number) \ ".pushsection .altinstructions,\"a\"\n" \ + ".p2align 2\n" \ ALTINSTR_ENTRY(feature, number) \ ".section .discard,\"a\",@progbits\n" \ DISCARD_ENTRY(number) \ ".section .altinstr_replacement, \"ax\"\n" \ + ".p2align 2\n" \ ALTINSTR_REPLACEMENT(newinstr, feature, number) \ ".popsection\n"
This is very similar to 137c59b9ff3f7a214f03b52d9c00a0a02374af1f "bug/x86/arm: Align bug_frames sections." On ARM and on x86 the C and assembler macros don't include any alignment information - hence they end up being the default byte granularity. On ARM32 it is paramount that the alignment is word-size (4) otherwise if one tries to use (uint32_t*) access (such as livepatch ELF relocations) we get a Data Abort. Enforcing .altinstructions (and also .altinstr_replacement for completness) to have the proper alignment across all architectures and in both C and x86 makes them all the same. On x86 the bloat-o-meter detects that with this change the file shrinks: add/remove: 1/0 grow/shrink: 0/2 up/down: 156/-367 (-211) function old new delta get_page_from_gfn - 156 +156 do_mmu_update 4578 4569 -9 do_mmuext_op 5604 5246 -358 Total: Before=3170439, After=3170228, chg -0.01% While on ARM 32/64: add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0) function old new delta Total: Before=822563, After=822563, chg +0.00% Signed-off-by: Konrad Rzeszutek Wilk <konrad@kernel.org> --- v2: First version --- xen/include/asm-arm/alternative.h | 4 ++++ xen/include/asm-x86/alternative.h | 2 ++ 2 files changed, 6 insertions(+)