[Qemu-devel] [PATCH] Fix multiboot compilation

[Qemu-devel] [PATCH] Fix multiboot compilation

[Alexander Graf]

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 <agraf@suse.de>
---
 pc-bios/optionrom/linuxboot.S |    8 ++++----
 pc-bios/optionrom/optionrom.h |   32 ++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 12 deletions(-)

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

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 <agraf@suse.de>
> ---
>  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
aurelien@aurel32.net                 http://www.aurel32.net