[PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[Jordan Justen]

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 4fcbdd2bc9..c92e90acfa 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -43,7 +43,6 @@ PLATFORMFILE=
 THREADNUMBER=1
 LAST_ARG=
 RUN_QEMU=no
-ENABLE_FLASH=no
 
 #
 # Pick a default tool type for a given OS
@@ -110,7 +109,7 @@ do
         break
         ;;
       --enable-flash)
-        ENABLE_FLASH=yes
+        # Ignore old option. We always enable flash.
         ;;
       *)
         BUILD_OPTIONS="$BUILD_OPTIONS $arg"
@@ -210,7 +209,10 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
                      awk '{print $2}')
   case $qemu_version in
     1.[6-9].*|[2-9].*.*|[1-9][0-9]*.*.*)
-      ENABLE_FLASH=yes
+      ;;
+    *)
+      echo qemu 1.6 or newer is required. detected: $qemu_version
+      exit 1
       ;;
   esac
 
@@ -258,15 +260,8 @@ fi
 
 
 if [[ "$RUN_QEMU" == "yes" ]]; then
-  if [[ ! -d $QEMU_FIRMWARE_DIR ]]; then
-    mkdir $QEMU_FIRMWARE_DIR
-  fi
-  ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin
-  if [[ "$ENABLE_FLASH" == "yes" ]]; then
-    QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin"
-  else
-    QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR"
-  fi
+  FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
+  QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
   if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
     QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
   fi
-- 
2.20.1

[PATCH 2/5] OvmfPkg/build.sh: Don't automatically -hda qemu parameter

[Jordan Justen]

This hasn't worked for me in quite some time. I always add a -cdrom
paramter, which causes build.sh to skip this.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index c92e90acfa..217abae683 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -262,9 +262,6 @@ fi
 if [[ "$RUN_QEMU" == "yes" ]]; then
   FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
   QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
-  if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
-    QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
-  fi
   echo Running: $QEMU_COMMAND "$@"
   $QEMU_COMMAND "$@"
   exit $?
-- 
2.20.1

[PATCH 3/5] OvmfPkg/build.sh: Move automatic TARGET_TOOLS detection later

[Jordan Justen]

If we are building for a non-native ARM, then we may need to select a
cross-compiler based on the -a paramter.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 99 +++++++++++++++++++++++++-----------------------
 1 file changed, 51 insertions(+), 48 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 217abae683..812441b9b1 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -40,58 +40,11 @@ ARCH_X64=no
 BUILDTARGET=DEBUG
 BUILD_OPTIONS=
 PLATFORMFILE=
+TARGET_TOOLS=
 THREADNUMBER=1
 LAST_ARG=
 RUN_QEMU=no
 
-#
-# Pick a default tool type for a given OS
-#
-TARGET_TOOLS=MYTOOLS
-case `uname` in
-  CYGWIN*)
-    echo Cygwin not fully supported yet.
-    ;;
-  Darwin*)
-    Major=$(uname -r | cut -f 1 -d '.')
-    # Major is Darwin version, not OS X version.
-    # OS X Yosemite 10.10.2 returns 14.
-    case $Major in
-      [156789])
-        echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
-        exit 1
-        ;;
-      10)
-        TARGET_TOOLS=XCODE32
-        ;;
-      1[12])
-        TARGET_TOOLS=XCLANG
-        ;;
-       *)
-        # Mavericks and future assume XCODE5 (clang + lldb)
-        TARGET_TOOLS=XCODE5
-        ;;
-    esac
-    ;;
-  Linux*)
-    gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
-    case $gcc_version in
-      [1-3].*|4.[0-7].*)
-        echo OvmfPkg requires GCC4.8 or later
-        exit 1
-        ;;
-      4.8.*)
-        TARGET_TOOLS=GCC48
-        ;;
-      4.9.*|6.[0-2].*)
-        TARGET_TOOLS=GCC49
-        ;;
-      *)
-        TARGET_TOOLS=GCC5
-        ;;
-    esac
-esac
-
 #
 # Scan command line to override defaults
 #
@@ -166,6 +119,56 @@ else
   BUILD_ROOT_ARCH=X64
 fi
 
+#
+# Pick a default tool type for a given OS
+#
+if [ -z "$TARGET_TOOLS" ]; then
+  TARGET_TOOLS=MYTOOLS
+  case `uname` in
+    CYGWIN*)
+      echo Cygwin not fully supported yet.
+      ;;
+    Darwin*)
+      Major=$(uname -r | cut -f 1 -d '.')
+      # Major is Darwin version, not OS X version.
+      # OS X Yosemite 10.10.2 returns 14.
+      case $Major in
+        [156789])
+          echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
+          exit 1
+          ;;
+        10)
+          TARGET_TOOLS=XCODE32
+          ;;
+        1[12])
+          TARGET_TOOLS=XCLANG
+          ;;
+         *)
+          # Mavericks and future assume XCODE5 (clang + lldb)
+          TARGET_TOOLS=XCODE5
+          ;;
+      esac
+      ;;
+    Linux*)
+      gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
+      case $gcc_version in
+        [1-3].*|4.[0-7].*)
+          echo OvmfPkg requires GCC4.8 or later
+          exit 1
+          ;;
+        4.8.*)
+          TARGET_TOOLS=GCC48
+          ;;
+        4.9.*|6.[0-2].*)
+          TARGET_TOOLS=GCC49
+          ;;
+        *)
+          TARGET_TOOLS=GCC5
+          ;;
+      esac
+  esac
+fi
+
 case $PROCESSOR in
   IA32)
     if [ -n "$QEMU_COMMAND" ]; then
-- 
2.20.1

Re: [edk2-devel] [PATCH 3/5] OvmfPkg/build.sh: Move automatic TARGET_TOOLS detection later

[Philippe Mathieu-Daudé]

Hi Jordan,

On 4/10/19 11:34 AM, Jordan Justen wrote:
> If we are building for a non-native ARM, then we may need to select a
> cross-compiler based on the -a paramter.

I am sorry but I am non-native English and I have hard time to
understand your comment.

OVMF targets x86 hardware, which is obviously not ARM. And I'm not aware
of another target architecture.  Are you talking about the host where
you build?

I am trying your series on a Aarch64 host but I fail at passing the
correct cross gcc. I set GCC5_IA32_PREFIX, GCC5_X64_PREFIX and GCC5_BIN
env vars, but the build script still run '"gcc"' instead of my cross one.

A more generic comment regarding your series: how do you use this script?

Thanks,

Phil.

> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  OvmfPkg/build.sh | 99 +++++++++++++++++++++++++-----------------------
>  1 file changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
> index 217abae683..812441b9b1 100755
> --- a/OvmfPkg/build.sh
> +++ b/OvmfPkg/build.sh
> @@ -40,58 +40,11 @@ ARCH_X64=no
>  BUILDTARGET=DEBUG
>  BUILD_OPTIONS=
>  PLATFORMFILE=
> +TARGET_TOOLS=
>  THREADNUMBER=1
>  LAST_ARG=
>  RUN_QEMU=no
>  
> -#
> -# Pick a default tool type for a given OS
> -#
> -TARGET_TOOLS=MYTOOLS
> -case `uname` in
> -  CYGWIN*)
> -    echo Cygwin not fully supported yet.
> -    ;;
> -  Darwin*)
> -    Major=$(uname -r | cut -f 1 -d '.')
> -    # Major is Darwin version, not OS X version.
> -    # OS X Yosemite 10.10.2 returns 14.
> -    case $Major in
> -      [156789])
> -        echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
> -        exit 1
> -        ;;
> -      10)
> -        TARGET_TOOLS=XCODE32
> -        ;;
> -      1[12])
> -        TARGET_TOOLS=XCLANG
> -        ;;
> -       *)
> -        # Mavericks and future assume XCODE5 (clang + lldb)
> -        TARGET_TOOLS=XCODE5
> -        ;;
> -    esac
> -    ;;
> -  Linux*)
> -    gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
> -    case $gcc_version in
> -      [1-3].*|4.[0-7].*)
> -        echo OvmfPkg requires GCC4.8 or later
> -        exit 1
> -        ;;
> -      4.8.*)
> -        TARGET_TOOLS=GCC48
> -        ;;
> -      4.9.*|6.[0-2].*)
> -        TARGET_TOOLS=GCC49
> -        ;;
> -      *)
> -        TARGET_TOOLS=GCC5
> -        ;;
> -    esac
> -esac
> -
>  #
>  # Scan command line to override defaults
>  #
> @@ -166,6 +119,56 @@ else
>    BUILD_ROOT_ARCH=X64
>  fi
>  
> +#
> +# Pick a default tool type for a given OS
> +#
> +if [ -z "$TARGET_TOOLS" ]; then
> +  TARGET_TOOLS=MYTOOLS
> +  case `uname` in
> +    CYGWIN*)
> +      echo Cygwin not fully supported yet.
> +      ;;
> +    Darwin*)
> +      Major=$(uname -r | cut -f 1 -d '.')
> +      # Major is Darwin version, not OS X version.
> +      # OS X Yosemite 10.10.2 returns 14.
> +      case $Major in
> +        [156789])
> +          echo OvmfPkg requires OS X Snow Leopard 10.6 or newer OS
> +          exit 1
> +          ;;
> +        10)
> +          TARGET_TOOLS=XCODE32
> +          ;;
> +        1[12])
> +          TARGET_TOOLS=XCLANG
> +          ;;
> +         *)
> +          # Mavericks and future assume XCODE5 (clang + lldb)
> +          TARGET_TOOLS=XCODE5
> +          ;;
> +      esac
> +      ;;
> +    Linux*)
> +      gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
> +      case $gcc_version in
> +        [1-3].*|4.[0-7].*)
> +          echo OvmfPkg requires GCC4.8 or later
> +          exit 1
> +          ;;
> +        4.8.*)
> +          TARGET_TOOLS=GCC48
> +          ;;
> +        4.9.*|6.[0-2].*)
> +          TARGET_TOOLS=GCC49
> +          ;;
> +        *)
> +          TARGET_TOOLS=GCC5
> +          ;;
> +      esac
> +  esac
> +fi
> +
>  case $PROCESSOR in
>    IA32)
>      if [ -n "$QEMU_COMMAND" ]; then
> 

Re: [edk2-devel] [PATCH 3/5] OvmfPkg/build.sh: Move automatic TARGET_TOOLS detection later

[Jordan Justen]

On 2019-04-10 08:37:22, Philippe Mathieu-Daudé wrote:
> Hi Jordan,
> 
> On 4/10/19 11:34 AM, Jordan Justen wrote:
> > If we are building for a non-native ARM, then we may need to select a
> > cross-compiler based on the -a paramter.
> 
> I am sorry but I am non-native English and I have hard time to
> understand your comment.
> 
> OVMF targets x86 hardware, which is obviously not ARM.

>From OvmfPkg/README:

"The Open Virtual Machine Firmware (OVMF) project aims to support
firmware for Virtual Machines using the edk2 code base."

This doesn't appear to indicate any particular architecture or VMM.
Nevertheless, ArmVirtPkg was created, so in effect, you are correct.

> And I'm not aware of another target architecture. Are you talking
> about the host where you build?

I mean the target. The patch looks for something like "-a AARCH64" on
the command line. If found, we try to build ArmVirtPkg/ArmVirtQemu.dsc
instead of OVMF.

It's a bit of a hack in that it will try to build the dsc from another
package. But, OvmfPkg/build.sh is a little more generic than one might
expect. For example, when I want to build MdeModulePkg, I usually just
run:

OvmfPkg/build.sh -p MdeModulePkg/MdeModulePkg.dsc

I find it convenient that the script sets up the build environment
automatically, and after the script is done, no changes persist in my
shell's environment.

> I am trying your series on a Aarch64 host but I fail at passing the
> correct cross gcc. I set GCC5_IA32_PREFIX, GCC5_X64_PREFIX and GCC5_BIN
> env vars, but the build script still run '"gcc"' instead of my cross one.
> 
> A more generic comment regarding your series: how do you use this script?

Yeah, you are right. The comments could be better. I'll try to improve
them.

Basically, these changes let me build ArmVirtPkg/ArmVirtQemu.dsc on
x86_64 Linux by simply added "-a AARCH64" or "-a ARM" parameteres to
this script. It also helped setup the qemu command line. Here are the
4 commands I used on x86_64 Linux to build and run ArmVirtPkg:

$ OvmfPkg/build.sh -a AARCH64

$ OvmfPkg/build.sh -a AARCH64 qemu -m 1024 -cpu cortex-a57 -M virt -net none

$ OvmfPkg/build.sh -a ARM

$ OvmfPkg/build.sh -a ARM qemu -m 1024 -cpu cortex-a15 -M virt -net none

I think the script might be usable on an ARM based build systems as
well, but it might require further tweaks. Ideally, if the build
system was AARCH64 based, then:

* OvmfPkg/build.sh: (with no params) would cause AARCH64 ArmVirtPkg to
  be built using a native compiler.

* OvmfPkg/build.sh -a ARM: would cause ARM ArmVirtPkg to be built
  using a cross compiler.

* OvmfPkg/build.sh -a X64: would cause X64 OVMF to be built using a
  cross compiler.

-Jordan

[PATCH 4/5] OvmfPkg/build.sh: Add AARCH64/ARM build and qemu support

[Jordan Justen]

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 72 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 812441b9b1..9d787452b5 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -35,6 +35,8 @@ fi
 # Configure defaults for various options
 #
 
+ARCH_AARCH64=no
+ARCH_ARM=no
 ARCH_IA32=no
 ARCH_X64=no
 BUILDTARGET=DEBUG
@@ -71,12 +73,14 @@ do
   else
     case $LAST_ARG in
       -a)
-        if [[ x"$arg" != x"IA32" && x"$arg" != x"X64" ]]; then
-          echo Unsupported processor architecture: $arg
-          echo Only IA32 or X64 is supported
-          exit 1
-        fi
-        eval ARCH_$arg=yes
+        case $arg in
+          AARCH64|ARM|IA32|X64)
+            eval ARCH_$arg=yes
+            ;;
+          *)
+            echo Unsupported processor architecture: $arg
+            exit 1
+	esac
         ;;
       -b)
         BUILDTARGET=$arg
@@ -99,7 +103,22 @@ do
   shift
 done
 
-if [[ "$ARCH_IA32" == "yes" && "$ARCH_X64" == "yes" ]]; then
+export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
+export GCC5_ARM_PREFIX=arm-linux-gnueabihf-
+
+if [[ "$ARCH_AARCH64" == "yes" ]]; then
+  PROCESSOR=AARCH64
+  Processor=AArch64
+  BUILD_OPTIONS="$BUILD_OPTIONS -a $PROCESSOR"
+  PLATFORM_BUILD_DIR=ArmVirtQemu-$PROCESSOR
+  BUILD_ROOT_ARCH=$PROCESSOR
+elif [[ "$ARCH_ARM" == "yes" ]]; then
+  PROCESSOR=ARM
+  Processor=Arm
+  BUILD_OPTIONS="$BUILD_OPTIONS -a $PROCESSOR"
+  PLATFORM_BUILD_DIR=ArmVirtQemu-$PROCESSOR
+  BUILD_ROOT_ARCH=$PROCESSOR
+elif [[ "$ARCH_IA32" == "yes" && "$ARCH_X64" == "yes" ]]; then
   PROCESSOR=IA32X64
   Processor=Ia32X64
   BUILD_OPTIONS="$BUILD_OPTIONS -a IA32 -a X64"
@@ -170,6 +189,22 @@ if [ -z "$TARGET_TOOLS" ]; then
 fi
 
 case $PROCESSOR in
+  AARCH64)
+    if [ -z "$QEMU_COMMAND" ]; then
+      #
+      # The user didn't set the QEMU_COMMAND variable.
+      #
+      QEMU_COMMAND=qemu-system-aarch64
+    fi
+    ;;
+  ARM)
+    if [ -z "$QEMU_COMMAND" ]; then
+      #
+      # The user didn't set the QEMU_COMMAND variable.
+      #
+      QEMU_COMMAND=qemu-system-aarch64
+    fi
+    ;;
   IA32)
     if [ -n "$QEMU_COMMAND" ]; then
       #
@@ -203,7 +238,14 @@ case $PROCESSOR in
 esac
 
 if [ -z "$PLATFORMFILE" ]; then
-  PLATFORMFILE=$WORKSPACE/OvmfPkg/OvmfPkg$Processor.dsc
+  case $PROCESSOR in
+    AARCH64|ARM)
+      PLATFORMFILE=$WORKSPACE/ArmVirtPkg/ArmVirtQemu.dsc
+      ;;
+    *)
+      PLATFORMFILE=$WORKSPACE/OvmfPkg/OvmfPkg$Processor.dsc
+      ;;
+  esac
 fi
 
 if [[ "$RUN_QEMU" == "yes" ]]; then
@@ -263,8 +305,15 @@ fi
 
 
 if [[ "$RUN_QEMU" == "yes" ]]; then
-  FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
-  QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
+  case $PROCESSOR in
+    AARCH64|ARM)
+      FIRMWARE_IMAGE="-bios $FV_DIR/QEMU_EFI.fd"
+      ;;
+    *)
+      FIRMWARE_IMAGE="-pflash $FV_DIR/OVMF.fd"
+      ;;
+  esac
+  QEMU_COMMAND="$QEMU_COMMAND $FIRMWARE_IMAGE"
   echo Running: $QEMU_COMMAND "$@"
   $QEMU_COMMAND "$@"
   exit $?
@@ -273,7 +322,6 @@ fi
 #
 # Build the edk2 OvmfPkg
 #
-echo Running edk2 build for OvmfPkg$Processor
+echo Running edk2 build for $PROCESSOR $PLATFORMFILE
 build -p $PLATFORMFILE $BUILD_OPTIONS -b $BUILDTARGET -t $TARGET_TOOLS -n $THREADNUMBER
 exit $?
-
-- 
2.20.1

[PATCH 5/5] OvmfPkg/build.sh: Add a cross compiler prefix for AARCH64/ARM

[Jordan Justen]

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
---
 OvmfPkg/build.sh | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
index 9d787452b5..e1a708942b 100755
--- a/OvmfPkg/build.sh
+++ b/OvmfPkg/build.sh
@@ -103,9 +103,6 @@ do
   shift
 done
 
-export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
-export GCC5_ARM_PREFIX=arm-linux-gnueabihf-
-
 if [[ "$ARCH_AARCH64" == "yes" ]]; then
   PROCESSOR=AARCH64
   Processor=AArch64
@@ -138,6 +135,23 @@ else
   BUILD_ROOT_ARCH=X64
 fi
 
+GCC_CROSS_PREFIX=
+gcc_machine=$(gcc -dumpmachine 2>&1 | awk -F - '{print $1}')
+case $gcc_machine in
+  x86_64)
+    case $BUILD_ROOT_ARCH in
+      AARCH64)
+	GCC_CROSS_PREFIX=aarch64-linux-gnu-
+        ;;
+      ARM)
+	GCC_CROSS_PREFIX=arm-linux-gnueabihf-
+        ;;
+    esac
+    ;;
+esac
+
+GCC=${GCC_CROSS_PREFIX}gcc
+
 #
 # Pick a default tool type for a given OS
 #
@@ -169,7 +183,7 @@ if [ -z "$TARGET_TOOLS" ]; then
       esac
       ;;
     Linux*)
-      gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
+      gcc_version=$($GCC -v 2>&1 | tail -1 | awk '{print $3}')
       case $gcc_version in
         [1-3].*|4.[0-7].*)
           echo OvmfPkg requires GCC4.8 or later
@@ -188,6 +202,10 @@ if [ -z "$TARGET_TOOLS" ]; then
   esac
 fi
 
+if [ -n "${GCC_CROSS_PREFIX}" ]; then
+  export ${TARGET_TOOLS}_${BUILD_ROOT_ARCH}_PREFIX=${GCC_CROSS_PREFIX}
+fi
+
 case $PROCESSOR in
   AARCH64)
     if [ -z "$QEMU_COMMAND" ]; then
-- 
2.20.1

Re: [edk2-devel] [PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[Laszlo Ersek]

Hi Jordan,

I've asked Phil (CC'd) to review this series in my stead. I'll be happy
to provide an ACK when Phil gives his R-b (if I should forget, please
ping me separately).

In the future, please include a cover letter; a cumulative diffstat
usually helps with the review. (I've had to check all patches to see
that only build.sh is being modified.)

Thanks,
Laszlo

On 04/10/19 11:34, Jordan Justen wrote:
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  OvmfPkg/build.sh | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
> index 4fcbdd2bc9..c92e90acfa 100755
> --- a/OvmfPkg/build.sh
> +++ b/OvmfPkg/build.sh
> @@ -43,7 +43,6 @@ PLATFORMFILE=
>  THREADNUMBER=1
>  LAST_ARG=
>  RUN_QEMU=no
> -ENABLE_FLASH=no
>  
>  #
>  # Pick a default tool type for a given OS
> @@ -110,7 +109,7 @@ do
>          break
>          ;;
>        --enable-flash)
> -        ENABLE_FLASH=yes
> +        # Ignore old option. We always enable flash.
>          ;;
>        *)
>          BUILD_OPTIONS="$BUILD_OPTIONS $arg"
> @@ -210,7 +209,10 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
>                       awk '{print $2}')
>    case $qemu_version in
>      1.[6-9].*|[2-9].*.*|[1-9][0-9]*.*.*)
> -      ENABLE_FLASH=yes
> +      ;;
> +    *)
> +      echo qemu 1.6 or newer is required. detected: $qemu_version
> +      exit 1
>        ;;
>    esac
>  
> @@ -258,15 +260,8 @@ fi
>  
>  
>  if [[ "$RUN_QEMU" == "yes" ]]; then
> -  if [[ ! -d $QEMU_FIRMWARE_DIR ]]; then
> -    mkdir $QEMU_FIRMWARE_DIR
> -  fi
> -  ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin
> -  if [[ "$ENABLE_FLASH" == "yes" ]]; then
> -    QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin"
> -  else
> -    QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR"
> -  fi
> +  FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
> +  QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
>    if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
>      QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
>    fi
> 

Re: [edk2-devel] [PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[Jordan Justen]

On 2019-04-10 07:16:34, Laszlo Ersek wrote:
> Hi Jordan,
> 
> I've asked Phil (CC'd) to review this series in my stead. I'll be happy
> to provide an ACK when Phil gives his R-b (if I should forget, please
> ping me separately).
> 
> In the future, please include a cover letter; a cumulative diffstat
> usually helps with the review. (I've had to check all patches to see
> that only build.sh is being modified.)

I did create a cover letter, but then I deleted it. I thought I was
making too much out of it...

>From what I gather, you don't find this script useful, and therefore I
sometimes wonder if I'm the only one that uses it. :)

-Jordan

Re: [edk2-devel] [PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[Laszlo Ersek]

On 04/10/19 22:18, Jordan Justen wrote:
> On 2019-04-10 07:16:34, Laszlo Ersek wrote:
>> Hi Jordan,
>>
>> I've asked Phil (CC'd) to review this series in my stead. I'll be happy
>> to provide an ACK when Phil gives his R-b (if I should forget, please
>> ping me separately).
>>
>> In the future, please include a cover letter; a cumulative diffstat
>> usually helps with the review. (I've had to check all patches to see
>> that only build.sh is being modified.)
> 
> I did create a cover letter, but then I deleted it. I thought I was
> making too much out of it...
Personally I appreciate a cover letter even if it consists only of:
- public repo / branch reference
- one sentence on the goal
- a CC list
- cumulative diffstat

It helps a lot with grouping emails too.

> From what I gather, you don't find this script useful,

That's correct. I prefer to use the "build" utility directly, and I set
it up in every situation where it is needed specifically for that
situation/environment. I have a number of scripts around "build".

Regarding testing / launching QEMU, for years I've considered the QEMU
command line a computer-only interface. I do 99% of my testing via
libvirt, and for the remaining 1%, I have humongous hand-written command
lines, stashed in a bunch of scripts. Whenever I have to change to that
directory on my laptop, I cringe.

It's possible that some "simple" command lines exist for QEMU, but for
me they've always turned out deceptive. They are simple to *start*, but
soon you find you have to add 7 more options, and now your command line
stretches multiple lines in your terminal and it's hard to navigate, so
you start writing a script in order to break each option to a separate
line with a backslash, but then some individual options are still too
long to fit into 80 characters, so you introduce shell variables for
abbreviations, etc etc etc grrrrr. :)

Libvirt puts all that to rest.

> and therefore I sometimes wonder if I'm the only one that uses it. :)

I can't tell, but even if you are, that should be no problem! It's just
that I might not have much time to review or discuss changes to it. I
really dislike this situation, but I'm at the point where I'm
practically *stealing* the time to discuss anything that's not directly
assigned to me. :(

Laszlo

Re: [edk2-devel] [PATCH 1/5] OvmfPkg/build.sh: Require QEMU 1.6 or newer and always enable flash

[Philippe Mathieu-Daudé]

On 4/10/19 11:34 AM, Jordan Justen wrote:
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> ---
>  OvmfPkg/build.sh | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/build.sh b/OvmfPkg/build.sh
> index 4fcbdd2bc9..c92e90acfa 100755
> --- a/OvmfPkg/build.sh
> +++ b/OvmfPkg/build.sh
> @@ -43,7 +43,6 @@ PLATFORMFILE=
>  THREADNUMBER=1
>  LAST_ARG=
>  RUN_QEMU=no
> -ENABLE_FLASH=no
>  
>  #
>  # Pick a default tool type for a given OS
> @@ -110,7 +109,7 @@ do
>          break
>          ;;
>        --enable-flash)
> -        ENABLE_FLASH=yes
> +        # Ignore old option. We always enable flash.
>          ;;
>        *)
>          BUILD_OPTIONS="$BUILD_OPTIONS $arg"
> @@ -210,7 +209,10 @@ if [[ "$RUN_QEMU" == "yes" ]]; then
>                       awk '{print $2}')
>    case $qemu_version in
>      1.[6-9].*|[2-9].*.*|[1-9][0-9]*.*.*)
> -      ENABLE_FLASH=yes
> +      ;;
> +    *)
> +      echo qemu 1.6 or newer is required. detected: $qemu_version
> +      exit 1
>        ;;
>    esac
>  
> @@ -258,15 +260,8 @@ fi
>  
>  
>  if [[ "$RUN_QEMU" == "yes" ]]; then
> -  if [[ ! -d $QEMU_FIRMWARE_DIR ]]; then
> -    mkdir $QEMU_FIRMWARE_DIR
> -  fi
> -  ln -sf $FV_DIR/OVMF.fd $QEMU_FIRMWARE_DIR/bios.bin
> -  if [[ "$ENABLE_FLASH" == "yes" ]]; then
> -    QEMU_COMMAND="$QEMU_COMMAND -pflash $QEMU_FIRMWARE_DIR/bios.bin"
> -  else
> -    QEMU_COMMAND="$QEMU_COMMAND -L $QEMU_FIRMWARE_DIR"
> -  fi
> +  FIRMWARE_IMAGE=$FV_DIR/OVMF.fd
> +  QEMU_COMMAND="$QEMU_COMMAND -pflash $FIRMWARE_IMAGE"
>    if [[ "$ADD_QEMU_HDA" == "yes" ]]; then
>      QEMU_COMMAND="$QEMU_COMMAND -hda fat:$BUILD_ROOT_ARCH"
>    fi
> 

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>