Re: [Qemu-devel] [PATCH] Fix multiboot compilation

[Aurelien Jarno]

On Wed, Jun 02, 2010 at 01:56:50AM +0200, Alexander Graf wrote:
> Commit dd4239d6574ca41c94fc0d0f77ddc728510ffc57 broke multiboot. It replaced 
> the
> instruction "rep insb (%dx), %es:(%edi)" by the binary output of
> "addr32 rep insb (%dx), %es:(%di)".
> 
> Linuxboot calls the respective helper function in a code16 section. So the
> original instruction was automatically translated to its "addr32" equivalent.
> For multiboot, we're running in code32 so gcc didn't add the "addr32" which
> breaks the instruction.
> 
> This patch splits that helper function in one which uses addr32 and one which
> does not, so everyone's happy.
> 
> The good news is that nobody probably cared so far. The bundled multiboot.bin
> binary was built before the change and is thus correct.
> 
> Please also put this patch into -stable.
> 
> Signed-off-by: Alexander Graf <address@hidden>
> ---
>  pc-bios/optionrom/linuxboot.S |    8 ++++----
>  pc-bios/optionrom/optionrom.h |   32 ++++++++++++++++++++++++--------
>  2 files changed, 28 insertions(+), 12 deletions(-)

Thanks, applied.

> diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S
> index 8aebe51..c109363 100644
> --- a/pc-bios/optionrom/linuxboot.S
> +++ b/pc-bios/optionrom/linuxboot.S
> @@ -106,10 +106,10 @@ copy_kernel:
>       /* We're now running in 16-bit CS, but 32-bit ES! */
>  
>       /* Load kernel and initrd */
> -     read_fw_blob(FW_CFG_KERNEL)
> -     read_fw_blob(FW_CFG_INITRD)
> -     read_fw_blob(FW_CFG_CMDLINE)
> -     read_fw_blob(FW_CFG_SETUP)
> +     read_fw_blob_addr32(FW_CFG_KERNEL)
> +     read_fw_blob_addr32(FW_CFG_INITRD)
> +     read_fw_blob_addr32(FW_CFG_CMDLINE)
> +     read_fw_blob_addr32(FW_CFG_SETUP)
>  
>       /* And now jump into Linux! */
>       mov             $0, %eax
> diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
> index 4dcb906..fbdd48a 100644
> --- a/pc-bios/optionrom/optionrom.h
> +++ b/pc-bios/optionrom/optionrom.h
> @@ -50,13 +50,7 @@
>       bswap           %eax
>  .endm
>  
> -/*
> - * Read a blob from the fw_cfg device.
> - * Requires _ADDR, _SIZE and _DATA values for the parameter.
> - *
> - * Clobbers: %eax, %edx, %es, %ecx, %edi
> - */
> -#define read_fw_blob(var)                            \
> +#define read_fw_blob_pre(var)                                \
>       read_fw         var ## _ADDR;                   \
>       mov             %eax, %edi;                     \
>       read_fw         var ## _SIZE;                   \
> @@ -65,10 +59,32 @@
>       mov             $BIOS_CFG_IOPORT_CFG, %edx;     \
>       outw            %ax, (%dx);                     \
>       mov             $BIOS_CFG_IOPORT_DATA, %dx;     \
> -     cld;                                            \
> +     cld
> +
> +/*
> + * Read a blob from the fw_cfg device.
> + * Requires _ADDR, _SIZE and _DATA values for the parameter.
> + *
> + * Clobbers: %eax, %edx, %es, %ecx, %edi
> + */
> +#define read_fw_blob(var)                            \
> +     read_fw_blob_pre(var);                          \
>       /* old as(1) doesn't like this insn so emit the bytes instead: \
>       rep insb        (%dx), %es:(%edi);              \
>       */                                              \
> +     .dc.b           0xf3,0x6c
> +
> +/*
> + * Read a blob from the fw_cfg device in forced addr32 mode.
> + * Requires _ADDR, _SIZE and _DATA values for the parameter.
> + *
> + * Clobbers: %eax, %edx, %es, %ecx, %edi
> + */
> +#define read_fw_blob_addr32(var)                             \
> +     read_fw_blob_pre(var);                          \
> +     /* old as(1) doesn't like this insn so emit the bytes instead: \
> +     addr32 rep insb (%dx), %es:(%edi);              \
> +     */                                              \
>       .dc.b           0x67,0xf3,0x6c
>  
>  #define OPTION_ROM_START                                     \
> -- 
> 1.6.0.2
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net