Re: [PATCH 58/79] OvmfPkg/Qemu: Fix various typos

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 12/9/19 1:55 PM, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 12/3/19 10:00 PM, Laszlo Ersek wrote:
>> On 12/03/19 17:15, Philippe Mathieu-Daude wrote:
>>> From: Antoine Coeur <coeur@gmx.fr>
>>>
>>> Fix various typos in comments and documentation.
>>>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
>>> ---
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf |  
>>> 4 ++--
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf        |  
>>> 4 ++--
>>>   OvmfPkg/AcpiPlatformDxe/Qemu.c                                   |  
>>> 4 ++--
>>>   OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c                          |  
>>> 2 +-
>>>   OvmfPkg/Library/DxePciLibI440FxQ35/PciLib.c                      |  
>>> 4 ++--
>>>   OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.c              |  
>>> 2 +-
>>>   OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c                      |  
>>> 2 +-
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbInfo.c                 |  
>>> 4 ++--
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c          |  
>>> 8 ++++----
>>>   OvmfPkg/QemuVideoDxe/Driver.c                                    |  
>>> 2 +-
>>>   OvmfPkg/QemuVideoDxe/VbeShim.asm                                 | 
>>> 14 +++++++-------
>>>   11 files changed, 25 insertions(+), 25 deletions(-)
>>>
> [...]>> diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.asm 
> b/OvmfPkg/QemuVideoDxe/VbeShim.asm
>>> index cb2a60d8278d..1d284b264124 100644
>>> --- a/OvmfPkg/QemuVideoDxe/VbeShim.asm
>>> +++ b/OvmfPkg/QemuVideoDxe/VbeShim.asm
>>> @@ -49,7 +49,7 @@ Handler:
>>>     je         ReadEdid
>>>     cmp        ah, 0x00
>>>     je         SetModeLegacy
>>> -  DebugLog   StrUnkownFunction
>>> +  DebugLog   StrUnknownFunction
>>>   Hang:
>>>     jmp        Hang
>>> @@ -93,7 +93,7 @@ GetModeInfo:
>>>     and        cx, ~0x4000 ; clear potentially set LFB bit in mode 
>>> number
>>>     cmp        cx, 0x00f1
>>>     je         KnownMode1
>>> -  DebugLog   StrUnkownMode
>>> +  DebugLog   StrUnknownMode
>>>     jmp        Hang
>>>   KnownMode1:
>>>     ; target (es:di) set on input
>>> @@ -155,7 +155,7 @@ SetMode:
>>>     cmp        bx, 0x40f1
>>>     je         KnownMode2
>>> -  DebugLog   StrUnkownMode
>>> +  DebugLog   StrUnknownMode
>>>     jmp        Hang
>>>   KnownMode2:
>>> @@ -203,7 +203,7 @@ SetModeLegacy:
>>>     je         KnownMode3
>>>     cmp        al, 0x12
>>>     je         KnownMode4
>>> -  DebugLog   StrUnkownMode
>>> +  DebugLog   StrUnknownMode
>>>     jmp        Hang
>>>   KnownMode3:
>>>     mov        al, 0x30
>>> @@ -252,7 +252,7 @@ StrExitSuccess:
>>>   StrExitUnsupported:
>>>     db 'Unsupported', 0x0a, 0
>>> -StrUnkownFunction:
>>> +StrUnknownFunction:
>>>     db 'Unknown Function', 0x0a, 0
>>>   StrEnterGetInfo:
>>> @@ -270,8 +270,8 @@ StrEnterSetMode:
>>>   StrEnterSetModeLegacy:
>>>     db 'SetModeLegacy', 0x0a, 0
>>> -StrUnkownMode:
>>> -  db 'Unkown Mode', 0x0a, 0
>>> +StrUnknownMode:
>>> +  db 'Unknown Mode', 0x0a, 0
>>>   StrGetPmCapabilities:
>>>     db 'GetPmCapabilities', 0x0a, 0
>>>
>>
>> If you modify "VbeShim.asm", then please re-run "VbeShim.sh", for
>> updating "VbeShim.h".
> 
> The DEBUG macro at the head of VbeShim.asm is commented out:
> 
>    ;%define DEBUG
> 
> 
> So the string "Unknown Mode" is not used.
> 
> Side note, running VbeShim.sh on Fedora 30 I get:
> 
> $ git diff --word-diff
> --- a/OvmfPkg/QemuVideoDxe/VbeShim.h
> +++ b/OvmfPkg/QemuVideoDxe/VbeShim.h
> @@ -523,13 +523,13 @@ STATIC CONST UINT8 mVbeShim[] = {
>    /* 0000020A cmp ax,0x4f02                  */  0x3D, 0x02, 0x4F,
>    /* 0000020D jz 0x269                       */  0x74, 0x5A,
>    /* 0000020F cmp ax,0x4f03                  */  0x3D, 0x03, 0x4F,
>    /* 00000212 jz [-word-]{+near+} 0x331                  */  0x0F, 
> 0x84, 0x1B, 0x01,
>    /* 00000216 cmp ax,0x4f10                  */  0x3D, 0x10, 0x4F,
>    /* 00000219 jz [-word-]{+near+} 0x336                  */  0x0F, 
> 0x84, 0x19, 0x01,
>    /* 0000021D cmp ax,0x4f15                  */  0x3D, 0x15, 0x4F,
>    /* 00000220 jz [-word-]{+near+} 0x338                  */  0x0F, 
> 0x84, 0x14, 0x01,
>    /* 00000224 cmp ah,0x0                     */  0x80, 0xFC, 0x00,
>    /* 00000227 jz [-word-]{+near+} 0x33a                  */  0x0F, 
> 0x84, 0x0F, 0x01,
>    /* 0000022B jmp short 0x22b                */  0xEB, 0xFE,
>    /* 0000022D push es                        */  0x06,
>    /* 0000022E push di                        */  0x57,
> @@ -547,7 +547,7 @@ STATIC CONST UINT8 mVbeShim[] = {
>    /* 0000023F pop ds                         */  0x1F,
>    /* 00000240 pop di                         */  0x5F,
>    /* 00000241 pop es                         */  0x07,
>    /* 00000242 jmp[-word-] 0x34c                      */  0xE9, 0x07, 0x01,
>    /* 00000245 push es                        */  0x06,
>    /* 00000246 push di                        */  0x57,
>    /* 00000247 push ds                        */  0x1E,
> @@ -568,7 +568,7 @@ STATIC CONST UINT8 mVbeShim[] = {
>    /* 00000263 pop ds                         */  0x1F,
>    /* 00000264 pop di                         */  0x5F,
>    /* 00000265 pop es                         */  0x07,
>    /* 00000266 jmp[-word-] 0x34c                      */  0xE9, 0xE3, 0x00,
>    /* 00000269 push dx                        */  0x52,
>    /* 0000026A push ax                        */  0x50,
>    /* 0000026B cmp bx,0x40f1                  */  0x81, 0xFB, 0xF1, 0x40,
> @@ -688,14 +688,14 @@ STATIC CONST UINT8 mVbeShim[] = {
>    /* 0000033C jz 0x345                       */  0x74, 0x07,
>    /* 0000033E cmp al,0x12                    */  0x3C, 0x12,
>    /* 00000340 jz 0x349                       */  0x74, 0x07,
>    /* 00000342 jmp[-word-] 0x22b                      */  0xE9, 0xE6, 0xFE,
>    /* 00000345 mov al,0x30                    */  0xB0, 0x30,
>    /* 00000347 jmp short 0x34b                */  0xEB, 0x02,
>    /* 00000349 mov al,0x20                    */  0xB0, 0x20,
>    /* 0000034B [-iretw-]{+iret+}                           */  0xCF,
>    /* 0000034C mov ax,0x4f                    */  0xB8, 0x4F, 0x00,
>    /* 0000034F [-iretw-]{+iret+}                           */  0xCF,
>    /* 00000350 mov ax,0x14f                   */  0xB8, 0x4F, 0x01,
>    /* 00000353 [-iretw-]{+iret+}                           */  0xCF,
> };
> #endif
> 
> The nasm packaged is:
> 
> $ nasm -v
> NASM version 2.13.03 compiled on Feb  2 2019
> 
> While NASM 2.12 is documented here:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Nasm-Setup
> 
> Should I downgrade it?

I forgot, to have VbeShim.sh succeed, I also have this local change:

-- >8 --
diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.sh 
b/OvmfPkg/QemuVideoDxe/VbeShim.sh
index aea28be35f5c..92b345befe1f 100755
--- a/OvmfPkg/QemuVideoDxe/VbeShim.sh
+++ b/OvmfPkg/QemuVideoDxe/VbeShim.sh
@@ -28,7 +28,7 @@ trap exit_handler EXIT
  #
  # Assemble the source file.
  #
-nasm -o "$STEM".bin -- "$STEM".asm
+nasm -fbin "$STEM".asm -o "$STEM".bin

  #
  # Disassemble it, in order to get a binary dump associated with the 
source.
---

Because I had this error:

$ sh -x OvmfPkg/QemuVideoDxe/VbeShim.sh
[...]
++ dirname -- OvmfPkg/QemuVideoDxe/VbeShim.sh
++ basename -- OvmfPkg/QemuVideoDxe/VbeShim.sh .sh
+ STEM=OvmfPkg/QemuVideoDxe/VbeShim
+ trap exit_handler EXIT
+ nasm -o OvmfPkg/QemuVideoDxe/VbeShim.bin -- 
OvmfPkg/QemuVideoDxe/VbeShim.asm
nasm: error: more than one input file specified
nasm: error: more than one input file specified
nasm: error: more than one input file specified
type `nasm -h' for help
+ exit_handler

Which is rather confuse, because the help is still recommending the 
current form:

$ nasm -h
usage: nasm [-@ response file] [-o outfile] [-f format] [-l listfile]
             [options...] [--] filename
     or nasm -v (or --v) for version info

     -t          assemble in SciTech TASM compatible mode
     -E (or -e)  preprocess only (writes output to stdout by default)
     -a          don't preprocess (assemble only)
     -M          generate Makefile dependencies on stdout
     -MG         d:o, missing files assumed generated
     -MF <file>  set Makefile dependency file
     -MD <file>  assemble and generate dependencies
     -MT <file>  dependency target name
     -MQ <file>  dependency target name (quoted)
     -MP         emit phony target

     -Z<file>    redirect error messages to file
     -s          redirect error messages to stdout

     -g          generate debugging information

     -F format   select a debugging format

     -gformat    same as -g -F format

     -o outfile  write output to an outfile

     -f format   select an output format

     -l listfile write listing to a listfile

     -I<path>    adds a pathname to the include file path
     -O<digit>   optimize branch offsets
                 -O0: No optimization
                 -O1: Minimal optimization
                 -Ox: Multipass optimization (default)

     -P<file>    pre-includes a file
     -D<macro>[=<value>] pre-defines a macro
     -U<macro>   undefines a macro
     -X<format>  specifies error reporting format (gnu or vc)
     -w+foo      enables warning foo (equiv. -Wfoo)
     -w-foo      disable warning foo (equiv. -Wno-foo)

     -w[+-]error[=foo] can be used to promote warnings to errors
     -h           show invocation summary and exit

--prefix,--postfix
                 these options prepend or append the given string
                 to all extern and global variables