Re: [RFC 2/5] x86/boot: create a C bundle for 32 bit boot code and use it

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Mon, Sep 9, 2024 at 11:10 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:

On 04.09.2024 16:56, Frediano Ziglio wrote:
> --- a/.gitignore
> +++ b/.gitignore
> @@ -251,8 +251,7 @@ xen/System.map
>  xen/arch/x86/boot/mkelf32
>  xen/arch/x86/boot/cmdline.S
>  xen/arch/x86/boot/reloc.S
> -xen/arch/x86/boot/*.bin
> -xen/arch/x86/boot/*.lnk
> +xen/arch/x86/boot/build32.*.lds

Please retain (largely) alphabetic sorting.

Changed locally.

 

> --- a/xen/arch/x86/boot/build32.lds
> +++ b/xen/arch/x86/boot/build32.lds.S
> @@ -15,22 +15,54 @@
>   * with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
> 
> -ENTRY(_start)
> +#undef ENTRY
> +
> +#ifdef FINAL
> +# define GAP 0
> +# define MULT 0
> +# define TEXT_START
> +#else
> +# define GAP 0x010200
> +# define MULT 1
> +# define TEXT_START 0x408020
> +#endif
> +# define DECLARE_IMPORT(name) name = . + (__LINE__ * MULT)
> +
> +ENTRY(dummy_start)
> 
>  SECTIONS
>  {
> -  /* Merge code and data into one section. */
> -  .text : {
> +  /* Merge code and read-only data into one section. */
> +  .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. */
> +        DECLARE_IMPORT(__base_relocs_start);
> +        DECLARE_IMPORT(__base_relocs_end);
> +        . = . + GAP;
>          *(.text)
>          *(.text.*)
> -        *(.data)
> -        *(.data.*)
>          *(.rodata)
>          *(.rodata.*)
> +  }
> +
> +  /* Writeable data sections. Check empty.
> +   * We collapse all into code section and we don't want it to be writeable. */
> +  .data : {
> +        *(.data)
> +        *(.data.*)
>          *(.bss)
>          *(.bss.*)
>    }
> -
> +  /DISCARD/ : {
> +       *(.comment)
> +       *(.comment.*)
> +       *(.note.*)
> +  }
>    /* Dynamic linkage sections.  Collected simply so we can check they're empty. */
>    .got : {
>          *(.got)
> @@ -49,6 +81,7 @@ SECTIONS
>    }
>    .rel : {
>          *(.rel.*)
> +        *(.data.rel.*)

This looks like you're mixing up .data.rel (a data section, where some items
may require relocation) and .rel.data (the relocation section for .data). If
you want all .data.* empty, this section should be empty, too. (I also don't
think this would ever take effect, due to the earlier *(.data.*).)

Removed this last hunk, yes, it's useless.

 

Jan

Thanks,

   Frediano