All of lore.kernel.org
 help / color / mirror / Atom feed
* [cip-dev][PATCH] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path
@ 2022-03-03 14:45 Srinuvasan A
  2022-03-03 14:54 ` Jan Kiszka
  0 siblings, 1 reply; 4+ messages in thread
From: Srinuvasan A @ 2022-03-03 14:45 UTC (permalink / raw)
  To: cip-dev; +Cc: jan.kiszka, Srinuvasan A

From: Srinuvasan A <srinuvasan_a@mentor.com>

Broke the normal boot part.
Fixed them.

-append takes arguments inside double quotes(""). If we escape
and pass the " along with the value the final command would form
like what we would expect.

qemu-system-x86_64 .... -append " root=/dev/sda console=ttyS0"

But when the shell parses it, due to how whitespace splitting works
for arguments passed via a variable, it parses the first quote(")
as argument 1 and root=dev/sda as argument 2 and messing up the
command.
It should ideally treat the whole " root=/dev/sda console=ttyS0" as
a single argument.

hence Maintaining argument splitting is complex. A simpler one for here is to
avoid BOOT_FILES and unrole the actual qemu call with its different
kernels into the secure, swupdate and normal path.

Signed-off-by: Srinuvasan A <srinuvasan_a@mentor.com>
---
 start-qemu.sh | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/start-qemu.sh b/start-qemu.sh
index a7e0588..d19cd98 100755
--- a/start-qemu.sh
+++ b/start-qemu.sh
@@ -121,22 +121,27 @@ if [ -n "${SECURE_BOOT}" ]; then
 			-global ICH9-LPC.disable_s3=1 \
 			-global isa-fdc.driveA= "
 
-		BOOT_FILES="-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
+		${QEMU_PATH}${QEMU} \
+			-m 1G -serial mon:stdio -netdev user,id=net \
+		        -drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
 			-drive if=pflash,format=raw,file=${ovmf_vars} \
-			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw"
+			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw ${QEMU_EXTRA_ARGS} "$@"
+
 elif [ -n "${SWUPDATE_BOOT}" ]; then
-		BOOT_FILES="-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
-			-bios OVMF.fd "
+		${QEMU_PATH}${QEMU} \
+			-m 1G -serial mon:stdio -netdev user,id=net \
+		        -drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
+			-bios OVMF.fd ${QEMU_EXTRA_ARGS} "$@"
+
 else
 		IMAGE_FILE=$(ls ${IMAGE_PREFIX}.ext4.img)
 
 		KERNEL_FILE=$(ls ${IMAGE_PREFIX}-vmlinu* | tail -1)
 		INITRD_FILE=$(ls ${IMAGE_PREFIX}-initrd.img* | tail -1)
 
-		BOOT_FILES="-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
+		${QEMU_PATH}${QEMU} \
+			-m 1G -serial mon:stdio -netdev user,id=net \
+                        -drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
 			-kernel ${KERNEL_FILE} -append "${KERNEL_CMDLINE}" \
-			-initrd ${INITRD_FILE}"
+			-initrd ${INITRD_FILE} ${QEMU_EXTRA_ARGS} "$@"
 fi
-${QEMU_PATH}${QEMU} \
-			-m 1G -serial mon:stdio -netdev user,id=net \
-			${BOOT_FILES} ${QEMU_EXTRA_ARGS} "$@"
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [cip-dev][PATCH] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path
  2022-03-03 14:45 [cip-dev][PATCH] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path Srinuvasan A
@ 2022-03-03 14:54 ` Jan Kiszka
  2022-03-04  8:03   ` [cip-dev][PATCH v2] " Srinuvasan A
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Kiszka @ 2022-03-03 14:54 UTC (permalink / raw)
  To: Srinuvasan A, cip-dev

On 03.03.22 15:45, Srinuvasan A wrote:
> From: Srinuvasan A <srinuvasan_a@mentor.com>
> 
> Broke the normal boot part.
> Fixed them.
> 
> -append takes arguments inside double quotes(""). If we escape
> and pass the " along with the value the final command would form
> like what we would expect.
> 
> qemu-system-x86_64 .... -append " root=/dev/sda console=ttyS0"
> 
> But when the shell parses it, due to how whitespace splitting works
> for arguments passed via a variable, it parses the first quote(")
> as argument 1 and root=dev/sda as argument 2 and messing up the
> command.
> It should ideally treat the whole " root=/dev/sda console=ttyS0" as
> a single argument.
> 
> hence Maintaining argument splitting is complex. A simpler one for here is to

"Hence maintaining..."

> avoid BOOT_FILES and unrole the actual qemu call with its different
> kernels into the secure, swupdate and normal path.
> 
> Signed-off-by: Srinuvasan A <srinuvasan_a@mentor.com>
> ---
>  start-qemu.sh | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/start-qemu.sh b/start-qemu.sh
> index a7e0588..d19cd98 100755
> --- a/start-qemu.sh
> +++ b/start-qemu.sh
> @@ -121,22 +121,27 @@ if [ -n "${SECURE_BOOT}" ]; then
>  			-global ICH9-LPC.disable_s3=1 \
>  			-global isa-fdc.driveA= "
>  
> -		BOOT_FILES="-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
> +		${QEMU_PATH}${QEMU} \
> +			-m 1G -serial mon:stdio -netdev user,id=net \
> +		        -drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
>  			-drive if=pflash,format=raw,file=${ovmf_vars} \
> -			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw"
> +			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw ${QEMU_EXTRA_ARGS} "$@"
> +
>  elif [ -n "${SWUPDATE_BOOT}" ]; then
> -		BOOT_FILES="-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
> -			-bios OVMF.fd "
> +		${QEMU_PATH}${QEMU} \
> +			-m 1G -serial mon:stdio -netdev user,id=net \
> +		        -drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
> +			-bios OVMF.fd ${QEMU_EXTRA_ARGS} "$@"
> +
>  else
>  		IMAGE_FILE=$(ls ${IMAGE_PREFIX}.ext4.img)
>  
>  		KERNEL_FILE=$(ls ${IMAGE_PREFIX}-vmlinu* | tail -1)
>  		INITRD_FILE=$(ls ${IMAGE_PREFIX}-initrd.img* | tail -1)
>  
> -		BOOT_FILES="-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
> +		${QEMU_PATH}${QEMU} \
> +			-m 1G -serial mon:stdio -netdev user,id=net \
> +                        -drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
>  			-kernel ${KERNEL_FILE} -append "${KERNEL_CMDLINE}" \
> -			-initrd ${INITRD_FILE}"
> +			-initrd ${INITRD_FILE} ${QEMU_EXTRA_ARGS} "$@"
>  fi
> -${QEMU_PATH}${QEMU} \
> -			-m 1G -serial mon:stdio -netdev user,id=net \
> -			${BOOT_FILES} ${QEMU_EXTRA_ARGS} "$@"

Functionally fine, but your indention is not consistent (mostly tabs,
not not always).

When you do a v2, you could use that chance and factor out the common
arguments into a variable (-m 1G -serial mon:stdio -netdev user,id=net).

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 4+ messages in thread
* [cip-dev][PATCH v2] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path
  2022-03-03 14:54 ` Jan Kiszka
@ 2022-03-04  8:03   ` Srinuvasan A
  2022-03-07  6:47     ` Jan Kiszka
  0 siblings, 1 reply; 4+ messages in thread
From: Srinuvasan A @ 2022-03-04  8:03 UTC (permalink / raw)
  To: cip-dev; +Cc: jan.kiszka, Srinuvasan A

From: Srinuvasan A <srinuvasan_a@mentor.com>

Broke the normal boot part.
Fixed them.

-append takes arguments inside double quotes(""). If we escape
and pass the " along with the value the final command would form
like what we would expect.

qemu-system-x86_64 .... -append " root=/dev/sda console=ttyS0"

But when the shell parses it, due to how whitespace splitting works
for arguments passed via a variable, it parses the first quote(")
as argument 1 and root=dev/sda as argument 2 and messing up the
command.
It should ideally treat the whole " root=/dev/sda console=ttyS0" as
a single argument.

Hence Maintaining argument splitting is complex. A simpler one for here is to
avoid BOOT_FILES and unrole the actual qemu call with its different
kernels into the secure, swupdate and normal path.

Signed-off-by: Srinuvasan A <srinuvasan_a@mentor.com>
---
 start-qemu.sh | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/start-qemu.sh b/start-qemu.sh
index a7e0588..4105d4e 100755
--- a/start-qemu.sh
+++ b/start-qemu.sh
@@ -114,6 +114,11 @@ fi
 
 shift 1
 
+QEMU_COMMON_OPTIONS=" \
+	-m 1G \
+	-serial mon:stdio \
+	-netdev user,id=net"
+
 if [ -n "${SECURE_BOOT}" ]; then
 		ovmf_code=${OVMF_CODE:-./build/tmp/deploy/images/qemu-amd64/OVMF/OVMF_CODE_4M.secboot.fd}
 		ovmf_vars=${OVMF_VARS:-./build/tmp/deploy/images/qemu-amd64/OVMF/OVMF_VARS_4M.snakeoil.fd}
@@ -121,22 +126,24 @@ if [ -n "${SECURE_BOOT}" ]; then
 			-global ICH9-LPC.disable_s3=1 \
 			-global isa-fdc.driveA= "
 
-		BOOT_FILES="-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
+		${QEMU_PATH}${QEMU} \
+			-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
 			-drive if=pflash,format=raw,file=${ovmf_vars} \
-			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw"
+			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
+
 elif [ -n "${SWUPDATE_BOOT}" ]; then
-		BOOT_FILES="-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
-			-bios OVMF.fd "
+		${QEMU_PATH}${QEMU} \
+			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
+			-bios OVMF.fd ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
+
 else
 		IMAGE_FILE=$(ls ${IMAGE_PREFIX}.ext4.img)
 
 		KERNEL_FILE=$(ls ${IMAGE_PREFIX}-vmlinu* | tail -1)
 		INITRD_FILE=$(ls ${IMAGE_PREFIX}-initrd.img* | tail -1)
 
-		BOOT_FILES="-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
+		${QEMU_PATH}${QEMU} \
+			-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
 			-kernel ${KERNEL_FILE} -append "${KERNEL_CMDLINE}" \
-			-initrd ${INITRD_FILE}"
+			-initrd ${INITRD_FILE} ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
 fi
-${QEMU_PATH}${QEMU} \
-			-m 1G -serial mon:stdio -netdev user,id=net \
-			${BOOT_FILES} ${QEMU_EXTRA_ARGS} "$@"
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [cip-dev][PATCH v2] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path
  2022-03-04  8:03   ` [cip-dev][PATCH v2] " Srinuvasan A
@ 2022-03-07  6:47     ` Jan Kiszka
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Kiszka @ 2022-03-07  6:47 UTC (permalink / raw)
  To: Srinuvasan A, cip-dev

On 04.03.22 09:03, Srinuvasan A wrote:
> From: Srinuvasan A <srinuvasan_a@mentor.com>
> 
> Broke the normal boot part.
> Fixed them.
> 
> -append takes arguments inside double quotes(""). If we escape
> and pass the " along with the value the final command would form
> like what we would expect.
> 
> qemu-system-x86_64 .... -append " root=/dev/sda console=ttyS0"
> 
> But when the shell parses it, due to how whitespace splitting works
> for arguments passed via a variable, it parses the first quote(")
> as argument 1 and root=dev/sda as argument 2 and messing up the
> command.
> It should ideally treat the whole " root=/dev/sda console=ttyS0" as
> a single argument.
> 
> Hence Maintaining argument splitting is complex. A simpler one for here is to
> avoid BOOT_FILES and unrole the actual qemu call with its different
> kernels into the secure, swupdate and normal path.
> 
> Signed-off-by: Srinuvasan A <srinuvasan_a@mentor.com>
> ---
>  start-qemu.sh | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/start-qemu.sh b/start-qemu.sh
> index a7e0588..4105d4e 100755
> --- a/start-qemu.sh
> +++ b/start-qemu.sh
> @@ -114,6 +114,11 @@ fi
>  
>  shift 1
>  
> +QEMU_COMMON_OPTIONS=" \
> +	-m 1G \
> +	-serial mon:stdio \
> +	-netdev user,id=net"
> +
>  if [ -n "${SECURE_BOOT}" ]; then
>  		ovmf_code=${OVMF_CODE:-./build/tmp/deploy/images/qemu-amd64/OVMF/OVMF_CODE_4M.secboot.fd}
>  		ovmf_vars=${OVMF_VARS:-./build/tmp/deploy/images/qemu-amd64/OVMF/OVMF_VARS_4M.snakeoil.fd}
> @@ -121,22 +126,24 @@ if [ -n "${SECURE_BOOT}" ]; then
>  			-global ICH9-LPC.disable_s3=1 \
>  			-global isa-fdc.driveA= "
>  
> -		BOOT_FILES="-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
> +		${QEMU_PATH}${QEMU} \
> +			-drive if=pflash,format=raw,unit=0,readonly=on,file=${ovmf_code} \
>  			-drive if=pflash,format=raw,file=${ovmf_vars} \
> -			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw"
> +			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
> +
>  elif [ -n "${SWUPDATE_BOOT}" ]; then
> -		BOOT_FILES="-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
> -			-bios OVMF.fd "
> +		${QEMU_PATH}${QEMU} \
> +			-drive file=${IMAGE_PREFIX}.wic.img,discard=unmap,if=none,id=disk,format=raw \
> +			-bios OVMF.fd ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
> +
>  else
>  		IMAGE_FILE=$(ls ${IMAGE_PREFIX}.ext4.img)
>  
>  		KERNEL_FILE=$(ls ${IMAGE_PREFIX}-vmlinu* | tail -1)
>  		INITRD_FILE=$(ls ${IMAGE_PREFIX}-initrd.img* | tail -1)
>  
> -		BOOT_FILES="-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
> +		${QEMU_PATH}${QEMU} \
> +			-drive file=${IMAGE_FILE},discard=unmap,if=none,id=disk,format=raw \
>  			-kernel ${KERNEL_FILE} -append "${KERNEL_CMDLINE}" \
> -			-initrd ${INITRD_FILE}"
> +			-initrd ${INITRD_FILE} ${QEMU_COMMON_OPTIONS} ${QEMU_EXTRA_ARGS} "$@"
>  fi
> -${QEMU_PATH}${QEMU} \
> -			-m 1G -serial mon:stdio -netdev user,id=net \
> -			${BOOT_FILES} ${QEMU_EXTRA_ARGS} "$@"

Thanks, applied. Further cleanup on top will follow.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-07  6:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 14:45 [cip-dev][PATCH] start-qemu.sh: Remove BOOT_FILES variable and call qemu directly in secure,swupdate and normal path Srinuvasan A
2022-03-03 14:54 ` Jan Kiszka
2022-03-04  8:03   ` [cip-dev][PATCH v2] " Srinuvasan A
2022-03-07  6:47     ` Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.