|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 1/5] x86/boot: create a C bundle for 32 bit boot code and use it
On Thu, Oct 17, 2024 at 02:31:19PM +0100, Frediano Ziglio wrote:
> The current method to include 32 bit C boot code is:
> - compile each function we want to use into a separate object file;
> - each function is compiled with -fpic option;
> - convert these object files to binary files. This operation removes GOP
> which we don't want in the executable;
> - a small assembly part in each file add the entry point;
> - code can't have external references, all possible variables are passed
> by value or pointer;
> - include these binary files in head.S.
>
> There are currently some limitations:
> - code is compiled separately, it's not possible to share a function
> (like memcpy) between different functions to use;
> - although code is compiled with -fpic there's no certainty there are
> no relocations, specifically data ones. This can lead into hard to
> find bugs;
> - it's hard to add a simple function;
> - having to pass external variables makes hard to do multiple things
> otherwise functions would require a lot of parameters so code would
> have to be split into multiple functions which is not easy.
>
> Current change extends the current process:
> - all object files are linked together before getting converted making
> possible to share code between the function we want to call;
> - a single object file is generated with all functions to use and
> exported symbols to easily call;
> - variables to use are declared in linker script and easily used inside
> C code. Declaring them manually could be annoying but makes also
> easier to check them. Using external pointers can be still an issue if
> they are not fixed. If an external symbol is not declared this gives a
> link error.
>
> Some details of the implementation:
> - C code is compiled with -fpic flags (as before);
> - object files from C code are linked together;
> - the single bundled object file is linked with 2 slightly different
> script files to generate 2 bundled object files;
> - the 2 bundled object files are converted to binary removing the need
> for global offset tables;
> - a Python script is used to generate assembly source from the 2
> binaries;
> - the single assembly file is compiled to generate final bundled object
> file;
> - to detect possible unwanted relocation in data/code code is generated
> with different addresses. This is enforced starting .text section at
> different positions and adding a fixed "gap" at the beginning.
> This makes sure code and data is position independent;
> - to detect used symbols in data/code symbols are placed in .text
> section at different offsets (based on the line in the linker script).
> This is needed as potentially a reference to a symbol is converted to
> a reference to the containing section so multiple symbols could be
> converted to reference to same symbol (section name) and we need to
> distinguish them;
> - --orphan-handling=error option to linker is used to make sure we
> account for all possible sections from C code;
>
> Current limitations:
> - the main one is the lack of support for 64 bit code. It would make
> sure that even the code used for 64 bit (at the moment EFI code) is
> code and data position independent. We cannot assume that code that
> came from code compiled for 32 bit and compiled for 64 bit is code and
> data position independent, different compiler options lead to
> different code/data.
>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> ---
> Changes since v2:
> - removed W^X limitation, allowing data;
> - added some comments to python script;
> - added extension to python script;
> - added header to generated assembly code from python script;
> - added starting symbol to generated assembly code from python script
> to make disassembly more clear;
> - other minor style changes to python script.
>
> Changes since v4:
> - add build32.final.lds build32.other.lds to targets macro;
> - place some comments over a rule, not inside;
> - simplified linking and producing binary rule;
> - renamed built_in_32 to built-in-32, coding style;
> - fix minor indentation;
> - put magic numbers in Makefile and propagate them;
> - minor variable cleanups in Python script;
> - add dependency to Python script.
>
> Changes since v5:
> - renamed "other" and "final" phases to "base" and "offset";
> - use if_changed macro to generate built-in-32.S.
> ---
> xen/arch/x86/boot/.gitignore | 5 +-
> xen/arch/x86/boot/Makefile | 47 +++-
> .../x86/boot/{build32.lds => build32.lds.S} | 35 ++-
> xen/arch/x86/boot/cmdline.c | 12 -
> xen/arch/x86/boot/head.S | 12 -
> xen/arch/x86/boot/reloc.c | 14 --
> xen/tools/combine_two_binaries.py | 220 ++++++++++++++++++
> 7 files changed, 292 insertions(+), 53 deletions(-)
> rename xen/arch/x86/boot/{build32.lds => build32.lds.S} (70%)
> create mode 100755 xen/tools/combine_two_binaries.py
>
> diff --git a/xen/arch/x86/boot/.gitignore b/xen/arch/x86/boot/.gitignore
> index a379db7988..7e85549751 100644
> --- a/xen/arch/x86/boot/.gitignore
> +++ b/xen/arch/x86/boot/.gitignore
> @@ -1,3 +1,4 @@
> /mkelf32
> -/*.bin
> -/*.lnk
> +/build32.*.lds
> +/built-in-32.*.bin
> +/built-in-32.*.map
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index 1199291d2b..5da19501be 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -1,4 +1,5 @@
> obj-bin-y += head.o
> +obj-bin-y += built-in-32.o
>
> obj32 := cmdline.32.o
> obj32 += reloc.32.o
> @@ -9,9 +10,6 @@ targets += $(obj32)
>
> obj32 := $(addprefix $(obj)/,$(obj32))
>
> -$(obj)/head.o: AFLAGS-y += -Wa$(comma)-I$(obj)
> -$(obj)/head.o: $(obj32:.32.o=.bin)
> -
> CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
> $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
> @@ -25,14 +23,47 @@ $(obj32): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
> $(obj)/%.32.o: $(src)/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> +orphan-handling-$(call ld-option,--orphan-handling=error) :=
> --orphan-handling=error
> LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) :=
> --no-warn-rwx-segments
> LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
> LD32 := $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT))
>
> -%.bin: %.lnk
> - $(OBJCOPY) -j .text -O binary $< $@
> +text_gap := 0x010200
> +text_diff := 0x408020
> +
> +$(obj)/build32.base.lds: AFLAGS-y += -DGAP=$(text_gap)
> -DTEXT_DIFF=$(text_diff)
> +$(obj)/build32.offset.lds: AFLAGS-y += -DGAP=$(text_gap)
> -DTEXT_DIFF=$(text_diff) -DFINAL
> +$(obj)/build32.base.lds $(obj)/build32.offset.lds: $(src)/build32.lds.S FORCE
> + $(call if_changed_dep,cpp_lds_S)
> +
> +targets += build32.offset.lds build32.base.lds
> +
> +# link all 32bit objects together
> +$(obj)/built-in-32.tmp.o: $(obj32)
> + $(LD32) -r -o $@ $^
> +
> +# link bundle with a given layout and extract a binary from it
> +$(obj)/built-in-32.%.bin: $(obj)/build32.%.lds $(obj)/built-in-32.tmp.o
> + $(LD32) $(orphan-handling-y) -N -T $< -Map $(@:bin=map) -o $(@:bin=o)
> $(filter %.o,$^)
> + $(OBJCOPY) -j .text -O binary $(@:bin=o) $@
> + rm -f $(@:bin=o)
> +
> +quiet_cmd_combine = GEN $@
> +cmd_combine = \
> + $(PYTHON) $(srctree)/tools/combine_two_binaries.py \
> + --gap=$(text_gap) --text-diff=$(text_diff) \
> + --script $(obj)/build32.offset.lds \
> + --bin1 $(obj)/built-in-32.base.bin \
> + --bin2 $(obj)/built-in-32.offset.bin \
> + --map $(obj)/built-in-32.offset.map \
> + --exports cmdline_parse_early,reloc \
> + --output $@
See xen/Rules.mk, for consistency the indentation should be done with
spaces when defining variables. That would also allow to align the
options.
> +
> +targets += built-in-32.S
>
> -%.lnk: %.32.o $(src)/build32.lds
> - $(LD32) -N -T $(filter %.lds,$^) -o $@ $<
> +# generate final object file combining and checking above binaries
> +$(obj)/built-in-32.S: $(obj)/built-in-32.base.bin
> $(obj)/built-in-32.offset.bin \
> + $(srctree)/tools/combine_two_binaries.py FORCE
Can you indent this using spaces also, so it's on the same column as
the ':'?
> + $(call if_changed,combine)
>
> -clean-files := *.lnk *.bin
> +clean-files := built-in-32.*.bin built-in-32.*.map build32.*.lds
> diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds.S
> similarity index 70%
> rename from xen/arch/x86/boot/build32.lds
> rename to xen/arch/x86/boot/build32.lds.S
> index 56edaa727b..e3f5e55261 100644
> --- a/xen/arch/x86/boot/build32.lds
> +++ b/xen/arch/x86/boot/build32.lds.S
> @@ -15,22 +15,47 @@
> * with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> -ENTRY(_start)
> +#ifdef FINAL
> +# undef GAP
> +# define GAP 0
> +# define MULT 0
> +# define TEXT_START
> +#else
> +# define MULT 1
> +# define TEXT_START TEXT_DIFF
> +#endif
In other places we use a single space between the hash and the define.
> +#define DECLARE_IMPORT(name) name = . + (__LINE__ * MULT)
> +
> +ENTRY(dummy_start)
>
> SECTIONS
> {
> /* Merge code and data into one section. */
> - .text : {
> + .text TEXT_START : {
> + /* Silence linker warning, we are not going to use it */
> + dummy_start = .;
> +
> + /* Declare below any symbol name needed.
> + * Each symbol should be on its own line.
> + * It looks like a tedious work but we make sure the things we use.
> + * Potentially they should be all variables. */
The style is wrong for the opening and closing comment delimiters.
I think it would be best if this was written in a more natural style.
/*
* Any symbols used should be declared below, this ensures which
* symbols are visible to the 32bit C boot code.
*/
I don't think you need to mention that each symbol should be on it's
own line.
Thanks, Roger.
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |