[PATCH 0/8] several Pkgs: FVB driver: correct NumOfLba vararg type in EraseBlocks()

[PATCH 0/8] several Pkgs: FVB driver: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

The same fix is replicated for every FVB driver. Details are in the
individual (mostly identical) commit messages.

Repo:   https://github.com/lersek/edk2.git
Branch: num_of_lba_uintn

Cc: Andrew Fish <afish@apple.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: David Wei <david.wei@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Mang Guo <mang.guo@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks
Laszlo

Laszlo Ersek (8):
  ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in
    EraseBlocks()
  DuetPkg/FvbRuntimeService: correct NumOfLba vararg type in
    EraseBlocks()
  EmulatorPkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in
    EraseBlocks()
  Nt32Pkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in
    EraseBlocks()
  OvmfPkg/EmuVariableFvbRuntimeDxe: correct NumOfLba vararg type in
    EraseBlocks()
  OvmfPkg/QemuFlashFvbServicesRuntimeDxe: correct NumOfLba vararg type
    in EraseBlocks()
  QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg type in
    EraseBlocks()
  Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in
    EraseBlocks()

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c       | 12 +++++++++---
 DuetPkg/FvbRuntimeService/FWBlockService.c                |  4 ++--
 EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c        |  4 ++--
 Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c            |  4 ++--
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c                    |  2 +-
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c   |  4 ++--
 QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c |  4 ++--
 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c              |  4 ++--
 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c               |  4 ++--
 9 files changed, 24 insertions(+), 18 deletions(-)

-- 
2.9.3

[PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

In addition, update the DEBUG macro invocation where NumOfLba is formatted
with the %d conversion specifier: UINTN values should be converted to
UINT64 and printed with %Lu or %Lx for portability between 32-bit and
64-bit.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 12a861267a86..3ea858f46ffb 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -628,10 +628,16 @@ FvbEraseBlocks (
     }
 
     // How many Lba blocks are we requested to erase?
-    NumOfLba = VA_ARG (Args, UINT32);
+    NumOfLba = VA_ARG (Args, UINTN);
 
     // All blocks must be within range
-    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
+    DEBUG ((
+      DEBUG_BLKIO,
+      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
+      Instance->StartLba + StartingLba,
+      (UINT64)NumOfLba,
+      Instance->Media.LastBlock
+      ));
     if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
       VA_END (Args);
       DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
@@ -656,7 +662,7 @@ FvbEraseBlocks (
     }
 
     // How many Lba blocks are we requested to erase?
-    NumOfLba = VA_ARG (Args, UINT32);
+    NumOfLba = VA_ARG (Args, UINTN);
 
     // Go through each one and erase it
     while (NumOfLba > 0) {
-- 
2.9.3

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Ard Biesheuvel]

On 18 May 2017 at 16:04, Laszlo Ersek <lersek@redhat.com> wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
>
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>
> In addition, update the DEBUG macro invocation where NumOfLba is formatted
> with the %d conversion specifier: UINTN values should be converted to
> UINT64 and printed with %Lu or %Lx for portability between 32-bit and
> 64-bit.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 12a861267a86..3ea858f46ffb 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -628,10 +628,16 @@ FvbEraseBlocks (
>      }
>
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>
>      // All blocks must be within range
> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> +    DEBUG ((
> +      DEBUG_BLKIO,
> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> +      Instance->StartLba + StartingLba,
> +      (UINT64)NumOfLba,
> +      Instance->Media.LastBlock
> +      ));
>      if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
>        VA_END (Args);
>        DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
> @@ -656,7 +662,7 @@ FvbEraseBlocks (
>      }
>
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>
>      // Go through each one and erase it
>      while (NumOfLba > 0) {
> --
> 2.9.3
>
>

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Jordan Justen]

On 2017-05-18 08:04:20, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
> > The variable argument list is a list of tuples. Each tuple describes a
> > range of LBAs to erase and consists of the following:
> > * An EFI_LBA that indicates the starting LBA
> > * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> In addition, update the DEBUG macro invocation where NumOfLba is formatted
> with the %d conversion specifier: UINTN values should be converted to
> UINT64 and printed with %Lu or %Lx for portability between 32-bit and
> 64-bit.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 12a861267a86..3ea858f46ffb 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -628,10 +628,16 @@ FvbEraseBlocks (
>      }
>  
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
>      // All blocks must be within range
> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> +    DEBUG ((
> +      DEBUG_BLKIO,
> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",

Notably this is still > 80 columns. Maybe?

      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
      "> LastBlock=%ld.\n",

I noticed the capital 'L' in "%Lu". Doesn't make a difference since
it's not hex...

The need for this debug message seems dubious to me, but I guess I
might not be familiar with situation.

Thanks for noticing and fixing this bug in all the EDK II drivers.

Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

> +      Instance->StartLba + StartingLba,
> +      (UINT64)NumOfLba,
> +      Instance->Media.LastBlock
> +      ));
>      if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
>        VA_END (Args);
>        DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
> @@ -656,7 +662,7 @@ FvbEraseBlocks (
>      }
>  
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
>      // Go through each one and erase it
>      while (NumOfLba > 0) {
> -- 
> 2.9.3
> 
> 

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/18/17 19:21, Jordan Justen wrote:
> On 2017-05-18 08:04:20, Laszlo Ersek wrote:
>> According to the PI spec, Volume 3,
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>>
>>> The variable argument list is a list of tuples. Each tuple describes a
>>> range of LBAs to erase and consists of the following:
>>> * An EFI_LBA that indicates the starting LBA
>>> * A UINTN that indicates the number of blocks to erase
>>
>> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>>
>> In this driver, the NumOfLba local variable is defined with type UINTN,
>> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>>
>> In addition, update the DEBUG macro invocation where NumOfLba is formatted
>> with the %d conversion specifier: UINTN values should be converted to
>> UINT64 and printed with %Lu or %Lx for portability between 32-bit and
>> 64-bit.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
>> index 12a861267a86..3ea858f46ffb 100644
>> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
>> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
>> @@ -628,10 +628,16 @@ FvbEraseBlocks (
>>      }
>>  
>>      // How many Lba blocks are we requested to erase?
>> -    NumOfLba = VA_ARG (Args, UINT32);
>> +    NumOfLba = VA_ARG (Args, UINTN);
>>  
>>      // All blocks must be within range
>> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
>> +    DEBUG ((
>> +      DEBUG_BLKIO,
>> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> 
> Notably this is still > 80 columns. Maybe?
> 
>       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
>       "> LastBlock=%ld.\n",

This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has
extremely long lines, the longest one (line 774) has 172 columns. I
broke up the above DEBUG so that it would at least fit in 120 chars per
line (which is the "second level" recommendation in the coding spec).

Also, the format string looked messy enough for me not to want to break
it up.

Finally, Leif has expressed a preference earlier for keeping DEBUG
format strings on one line, if memory serves. (I certainly don't follow
that in OvmfPkg and ArmVirtPkg, but ArmPlatformPkg is not my co-turf :) )

I think I'd like to leave the format string like this.

> 
> I noticed the capital 'L' in "%Lu". Doesn't make a difference since
> it's not hex...

PrintLib handes "l" and "L" interchangeably. In
BasePrintLibSPrintMarker(), file
"MdePkg/Library/BasePrintLib/PrintLibInternal.c", we have

        case 'L':
        case 'l':
          Flags |= LONG_TYPE;
          break;

and all hex characters are always printed upper-case (see "mHexStr").

Instead, I use the "L" length modifier rather than "l" because:
- in ISO C, "l" (in %lx, %ld, %lu) stands for "long", and "L" is not
valid for integers,
- while in edk2, both "l" and "L" stand for 64-bit (which is a different
question from "long").

In other words, "l" has conflicting meanings between ISO C and edk2
(long vs. 64-bit), while "L" is uniquely defined (it is undefined in ISO
C, for integers, and in edk2 it stands for 64-bit).

> The need for this debug message seems dubious to me, but I guess I
> might not be familiar with situation.
> 
> Thanks for noticing and fixing this bug in all the EDK II drivers.

For noticing the bug (outside of OvmfPkg/EmuVariableFvbRuntimeDxe, where
I originally fixed it), the credit goes to you :) The patches have the
right Reported-by: tag.

> 
> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Thanks!
Laszlo

>> +      Instance->StartLba + StartingLba,
>> +      (UINT64)NumOfLba,
>> +      Instance->Media.LastBlock
>> +      ));
>>      if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
>>        VA_END (Args);
>>        DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
>> @@ -656,7 +662,7 @@ FvbEraseBlocks (
>>      }
>>  
>>      // How many Lba blocks are we requested to erase?
>> -    NumOfLba = VA_ARG (Args, UINT32);
>> +    NumOfLba = VA_ARG (Args, UINTN);
>>  
>>      // Go through each one and erase it
>>      while (NumOfLba > 0) {
>> -- 
>> 2.9.3
>>
>>

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Jordan Justen]

On 2017-05-18 12:29:09, Laszlo Ersek wrote:
> On 05/18/17 19:21, Jordan Justen wrote:
> > On 2017-05-18 08:04:20, Laszlo Ersek wrote:
> >>      // All blocks must be within range
> >> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> >> +    DEBUG ((
> >> +      DEBUG_BLKIO,
> >> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> > 
> > Notably this is still > 80 columns. Maybe?
> > 
> >       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
> >       "> LastBlock=%ld.\n",
> 
> This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has
> extremely long lines, the longest one (line 774) has 172 columns. I
> broke up the above DEBUG so that it would at least fit in 120 chars per
> line (which is the "second level" recommendation in the coding spec).

Personally, I don't agree with that secondary 120 char rule. If we
ever get the style guide into an 'open source' process, I'd like to
suggest removing it. (But, it'll probably get shot down. :\ )

> Also, the format string looked messy enough for me not to want to break
> it up.
> 
> Finally, Leif has expressed a preference earlier for keeping DEBUG
> format strings on one line, if memory serves. (I certainly don't follow
> that in OvmfPkg and ArmVirtPkg, but ArmPlatformPkg is not my co-turf :) )

Ah. I guess it is fine for a package maintainer to occasionally decide
to bend the rules for their package. Hopefully it would not eventually
lead to the style migrating into other packages though.

-Jordan

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Leif Lindholm]

On Thu, May 18, 2017 at 01:44:58PM -0700, Jordan Justen wrote:
> On 2017-05-18 12:29:09, Laszlo Ersek wrote:
> > On 05/18/17 19:21, Jordan Justen wrote:
> > > On 2017-05-18 08:04:20, Laszlo Ersek wrote:
> > >>      // All blocks must be within range
> > >> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> > >> +    DEBUG ((
> > >> +      DEBUG_BLKIO,
> > >> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> > > 
> > > Notably this is still > 80 columns. Maybe?
> > > 
> > >       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
> > >       "> LastBlock=%ld.\n",
> > 
> > This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has
> > extremely long lines, the longest one (line 774) has 172 columns. I
> > broke up the above DEBUG so that it would at least fit in 120 chars per
> > line (which is the "second level" recommendation in the coding spec).
> 
> Personally, I don't agree with that secondary 120 char rule. If we
> ever get the style guide into an 'open source' process, I'd like to
> suggest removing it. (But, it'll probably get shot down. :\ )

Oh, I'm all for that one. And the style guide is definitely in need of
a shake-up.

But I consider violating line length restrictions less bad than making
user-(or in this case developer)-visible strings harder to search for.

> Ah. I guess it is fine for a package maintainer to occasionally decide
> to bend the rules for their package.

For this to be bending, it would require the 120-character rule to not
exist.

/
    Leif

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Jordan Justen]

On 2017-05-19 03:07:56, Leif Lindholm wrote:
> On Thu, May 18, 2017 at 01:44:58PM -0700, Jordan Justen wrote:
> > On 2017-05-18 12:29:09, Laszlo Ersek wrote:
> > > On 05/18/17 19:21, Jordan Justen wrote:
> > > > On 2017-05-18 08:04:20, Laszlo Ersek wrote:
> > > >>      // All blocks must be within range
> > > >> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> > > >> +    DEBUG ((
> > > >> +      DEBUG_BLKIO,
> > > >> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> > > > 
> > > > Notably this is still > 80 columns. Maybe?
> > > > 
> > > >       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
> > > >       "> LastBlock=%ld.\n",
> > > 
> > > This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has
> > > extremely long lines, the longest one (line 774) has 172 columns. I
> > > broke up the above DEBUG so that it would at least fit in 120 chars per
> > > line (which is the "second level" recommendation in the coding spec).
> > 
> > Personally, I don't agree with that secondary 120 char rule. If we
> > ever get the style guide into an 'open source' process, I'd like to
> > suggest removing it. (But, it'll probably get shot down. :\ )
> 
> Oh, I'm all for that one. And the style guide is definitely in need of
> a shake-up.
> 
> But I consider violating line length restrictions less bad than making
> user-(or in this case developer)-visible strings harder to search for.

While of less concern, I would say the style guide should recommend
that logged debug messages should attempt to be less than 80 chars.
That could help here.

> > Ah. I guess it is fine for a package maintainer to occasionally decide
> > to bend the rules for their package.
> 
> For this to be bending, it would require the 120-character rule to not
> exist.

Yeah. It is very silly to say, we prefer 80 chars, but if it's too
hard then 120 can be used. :)

The latest coding standard says: "Preferably, limit line lengths to 80
columns or less. When this doesn’t leave sufficient space for a good
postfix style comment, extend the line to a total of 120 columns."

This seems to imply that the extension beyond 80 chars is reserved for
postfix comments. This seems like a poor reason to push past 80. It
would be better to comment above the line instead.

-Jordan

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Leif Lindholm]

On Fri, May 19, 2017 at 09:59:06AM -0700, Jordan Justen wrote:
> On 2017-05-19 03:07:56, Leif Lindholm wrote:
> > On Thu, May 18, 2017 at 01:44:58PM -0700, Jordan Justen wrote:
> > > On 2017-05-18 12:29:09, Laszlo Ersek wrote:
> > > > On 05/18/17 19:21, Jordan Justen wrote:
> > > > > On 2017-05-18 08:04:20, Laszlo Ersek wrote:
> > > > >>      // All blocks must be within range
> > > > >> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> > > > >> +    DEBUG ((
> > > > >> +      DEBUG_BLKIO,
> > > > >> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> > > > > 
> > > > > Notably this is still > 80 columns. Maybe?
> > > > > 
> > > > >       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) "
> > > > >       "> LastBlock=%ld.\n",
> > > > 
> > > > This file ("ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c") has
> > > > extremely long lines, the longest one (line 774) has 172 columns. I
> > > > broke up the above DEBUG so that it would at least fit in 120 chars per
> > > > line (which is the "second level" recommendation in the coding spec).
> > > 
> > > Personally, I don't agree with that secondary 120 char rule. If we
> > > ever get the style guide into an 'open source' process, I'd like to
> > > suggest removing it. (But, it'll probably get shot down. :\ )
> > 
> > Oh, I'm all for that one. And the style guide is definitely in need of
> > a shake-up.
> > 
> > But I consider violating line length restrictions less bad than making
> > user-(or in this case developer)-visible strings harder to search for.
> 
> While of less concern, I would say the style guide should recommend
> that logged debug messages should attempt to be less than 80 chars.
> That could help here.

Yes - had that string appeared as a new entity (instead of a
modification of existing code, with Laszlo being the good citizen to
make sure the modified lines complied as well as possible), that would
most likely have been my feedback.

> > > Ah. I guess it is fine for a package maintainer to occasionally decide
> > > to bend the rules for their package.
> > 
> > For this to be bending, it would require the 120-character rule to not
> > exist.
> 
> Yeah. It is very silly to say, we prefer 80 chars, but if it's too
> hard then 120 can be used. :)
> 
> The latest coding standard says: "Preferably, limit line lengths to 80
> columns or less. When this doesn’t leave sufficient space for a good
> postfix style comment, extend the line to a total of 120 columns."
> 
> This seems to imply that the extension beyond 80 chars is reserved for
> postfix comments. This seems like a poor reason to push past 80. It
> would be better to comment above the line instead.

Most definitely.

/
    Leif
 

Re: [PATCH 1/8] ArmPlatformPkg/NorFlashDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> In addition, update the DEBUG macro invocation where NumOfLba is formatted
> with the %d conversion specifier: UINTN values should be converted to
> UINT64 and printed with %Lu or %Lx for portability between 32-bit and
> 64-bit.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 12a861267a86..3ea858f46ffb 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -628,10 +628,16 @@ FvbEraseBlocks (
>      }
>  
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
>      // All blocks must be within range
> -    DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n", Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
> +    DEBUG ((
> +      DEBUG_BLKIO,
> +      "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%Lu - 1 ) > LastBlock=%ld.\n",
> +      Instance->StartLba + StartingLba,
> +      (UINT64)NumOfLba,
> +      Instance->Media.LastBlock
> +      ));
>      if ((NumOfLba == 0) || ((Instance->StartLba + StartingLba + NumOfLba - 1) > Instance->Media.LastBlock)) {
>        VA_END (Args);
>        DEBUG ((EFI_D_ERROR, "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
> @@ -656,7 +662,7 @@ FvbEraseBlocks (
>      }
>  
>      // How many Lba blocks are we requested to erase?
> -    NumOfLba = VA_ARG (Args, UINT32);
> +    NumOfLba = VA_ARG (Args, UINTN);
>  
>      // Go through each one and erase it
>      while (NumOfLba > 0) {
> 

Thanks all for the feedback, this is commit ce69cc776cfc.

[PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    unbuilt, untested

 DuetPkg/FvbRuntimeService/FWBlockService.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/DuetPkg/FvbRuntimeService/FWBlockService.c b/DuetPkg/FvbRuntimeService/FWBlockService.c
index e0ddbd53121b..bc663273d4ce 100644
--- a/DuetPkg/FvbRuntimeService/FWBlockService.c
+++ b/DuetPkg/FvbRuntimeService/FWBlockService.c
@@ -991,7 +991,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
@@ -1011,7 +1011,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while (NumOfLba > 0) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
-- 
2.9.3

Re: [PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

Ray, can you please review this patch?

Thanks,
Laszlo

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  DuetPkg/FvbRuntimeService/FWBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/DuetPkg/FvbRuntimeService/FWBlockService.c b/DuetPkg/FvbRuntimeService/FWBlockService.c
> index e0ddbd53121b..bc663273d4ce 100644
> --- a/DuetPkg/FvbRuntimeService/FWBlockService.c
> +++ b/DuetPkg/FvbRuntimeService/FWBlockService.c
> @@ -991,7 +991,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> @@ -1011,7 +1011,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
> 

Re: [PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba vararg type in EraseBlocks()

[Wu, Hao A]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Thursday, May 18, 2017 11:04 PM
> To: edk2-devel-01
> Cc: Ni, Ruiyu; Justen, Jordan L
> Subject: [edk2] [PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba
> vararg type in EraseBlocks()
> 
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
> > The variable argument list is a list of tuples. Each tuple describes a
> > range of LBAs to erase and consists of the following:
> > * An EFI_LBA that indicates the starting LBA
> > * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  DuetPkg/FvbRuntimeService/FWBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/DuetPkg/FvbRuntimeService/FWBlockService.c
> b/DuetPkg/FvbRuntimeService/FWBlockService.c
> index e0ddbd53121b..bc663273d4ce 100644
> --- a/DuetPkg/FvbRuntimeService/FWBlockService.c
> +++ b/DuetPkg/FvbRuntimeService/FWBlockService.c
> @@ -991,7 +991,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      //
>      // Check input parameters
> @@ -1011,7 +1011,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba,
> mFvbModuleGlobal, EfiGoneVirtual ());
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/27/17 02:54, Wu, Hao A wrote:
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Commit d98e939f4fd7.

Thanks!
Laszlo



>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
>> Ersek
>> Sent: Thursday, May 18, 2017 11:04 PM
>> To: edk2-devel-01
>> Cc: Ni, Ruiyu; Justen, Jordan L
>> Subject: [edk2] [PATCH 2/8] DuetPkg/FvbRuntimeService: correct NumOfLba
>> vararg type in EraseBlocks()
>>
>> According to the PI spec, Volume 3,
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>>
>>> The variable argument list is a list of tuples. Each tuple describes a
>>> range of LBAs to erase and consists of the following:
>>> * An EFI_LBA that indicates the starting LBA
>>> * A UINTN that indicates the number of blocks to erase
>>
>> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>>
>> In this driver, the NumOfLba local variable is defined with type UINTN,
>> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     unbuilt, untested
>>
>>  DuetPkg/FvbRuntimeService/FWBlockService.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/DuetPkg/FvbRuntimeService/FWBlockService.c
>> b/DuetPkg/FvbRuntimeService/FWBlockService.c
>> index e0ddbd53121b..bc663273d4ce 100644
>> --- a/DuetPkg/FvbRuntimeService/FWBlockService.c
>> +++ b/DuetPkg/FvbRuntimeService/FWBlockService.c
>> @@ -991,7 +991,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      //
>>      // Check input parameters
>> @@ -1011,7 +1011,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      while (NumOfLba > 0) {
>>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba,
>> mFvbModuleGlobal, EfiGoneVirtual ());
>> --
>> 2.9.3
>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

[PATCH 3/8] EmulatorPkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Andrew Fish <afish@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    build-tested for Ia32 and X64, not runtime tested

 EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c b/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
index 44df1172b12d..fdffca2c9449 100644
--- a/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
+++ b/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
@@ -906,7 +906,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
@@ -926,7 +926,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while (NumOfLba > 0) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
-- 
2.9.3

Re: [PATCH 3/8] EmulatorPkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     build-tested for Ia32 and X64, not runtime tested
> 
>  EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c b/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
> index 44df1172b12d..fdffca2c9449 100644
> --- a/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
> +++ b/EmulatorPkg/FvbServicesRuntimeDxe/FWBlockService.c
> @@ -906,7 +906,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> @@ -926,7 +926,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
> 

Commit 1ee0e6532fb1.

[PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    unbuilt, untested

 Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
index 3400516f0c3c..03f1f44dafde 100644
--- a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
+++ b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
@@ -945,7 +945,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
@@ -965,7 +965,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while (NumOfLba > 0) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
-- 
2.9.3

Re: [PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

Ray, can you please review this patch too?

Thanks,
Laszlo

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> index 3400516f0c3c..03f1f44dafde 100644
> --- a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> +++ b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> @@ -945,7 +945,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> @@ -965,7 +965,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
> 

Re: [PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Wu, Hao A]

Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Thursday, May 18, 2017 11:04 PM
> To: edk2-devel-01
> Cc: Ni, Ruiyu; Justen, Jordan L
> Subject: [edk2] [PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct
> NumOfLba vararg type in EraseBlocks()
> 
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
> > The variable argument list is a list of tuples. Each tuple describes a
> > range of LBAs to erase and consists of the following:
> > * An EFI_LBA that indicates the starting LBA
> > * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> index 3400516f0c3c..03f1f44dafde 100644
> --- a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> +++ b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
> @@ -945,7 +945,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      //
>      // Check input parameters
> @@ -965,7 +965,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba,
> mFvbModuleGlobal, EfiGoneVirtual ());
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/27/17 02:54, Wu, Hao A wrote:
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Commit d0d7289cce48.

Thanks!
Laszlo

> 
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
>> Ersek
>> Sent: Thursday, May 18, 2017 11:04 PM
>> To: edk2-devel-01
>> Cc: Ni, Ruiyu; Justen, Jordan L
>> Subject: [edk2] [PATCH 4/8] Nt32Pkg/FvbServicesRuntimeDxe: correct
>> NumOfLba vararg type in EraseBlocks()
>>
>> According to the PI spec, Volume 3,
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>>
>>> The variable argument list is a list of tuples. Each tuple describes a
>>> range of LBAs to erase and consists of the following:
>>> * An EFI_LBA that indicates the starting LBA
>>> * A UINTN that indicates the number of blocks to erase
>>
>> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>>
>> In this driver, the NumOfLba local variable is defined with type UINTN,
>> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     unbuilt, untested
>>
>>  Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
>> b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
>> index 3400516f0c3c..03f1f44dafde 100644
>> --- a/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
>> +++ b/Nt32Pkg/FvbServicesRuntimeDxe/FWBlockService.c
>> @@ -945,7 +945,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      //
>>      // Check input parameters
>> @@ -965,7 +965,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      while (NumOfLba > 0) {
>>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba,
>> mFvbModuleGlobal, EfiGoneVirtual ());
>> --
>> 2.9.3
>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

[PATCH 5/8] OvmfPkg/EmuVariableFvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
index 7a6d3153ec8c..9f9babc9cc33 100644
--- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
+++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
@@ -340,7 +340,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
-- 
2.9.3

Re: [PATCH 5/8] OvmfPkg/EmuVariableFvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> index 7a6d3153ec8c..9f9babc9cc33 100644
> --- a/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> +++ b/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c
> @@ -340,7 +340,7 @@ FvbProtocolEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> 

Commit 9c4eef656fa3.

[PATCH 6/8] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
index ff27c1100c01..558b395dff4a 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -645,7 +645,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
@@ -665,7 +665,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while (NumOfLba > 0) {
       Status = QemuFlashEraseBlock (StartingLba);
-- 
2.9.3

Re: [PATCH 6/8] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> index ff27c1100c01..558b395dff4a 100644
> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> @@ -645,7 +645,7 @@ FvbProtocolEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> @@ -665,7 +665,7 @@ FvbProtocolEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while (NumOfLba > 0) {
>        Status = QemuFlashEraseBlock (StartingLba);
> 

Commit 38292c08728d.

[PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    unbuilt, untested

 QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
index dbb5512f386e..0e7a7b79a1cf 100644
--- a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
+++ b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
@@ -1211,7 +1211,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters
@@ -1235,7 +1235,7 @@ Returns:
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while (NumOfLba > 0) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
-- 
2.9.3

Re: [PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

Mike or Kelly, can one of you please review this patch?

Thanks
Laszlo

On 05/18/17 17:04, Laszlo Ersek wrote:
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kelly Steele <kelly.steele@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> index dbb5512f386e..0e7a7b79a1cf 100644
> --- a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> +++ b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> @@ -1211,7 +1211,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters
> @@ -1235,7 +1235,7 @@ Returns:
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal, EfiGoneVirtual ());
> 

Re: [PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg type in EraseBlocks()

[Kinney, Michael D]

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, May 18, 2017 8:04 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>
> Subject: [edk2] [PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg
> type in EraseBlocks()
> 
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
> > The variable argument list is a list of tuples. Each tuple describes a
> > range of LBAs to erase and consists of the following:
> > * An EFI_LBA that indicates the starting LBA
> > * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kelly Steele <kelly.steele@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> index dbb5512f386e..0e7a7b79a1cf 100644
> --- a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> +++ b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
> @@ -1211,7 +1211,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      //
>      // Check input parameters
> @@ -1235,7 +1235,7 @@ Returns:
>        break;
>      }
> 
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
> 
>      while (NumOfLba > 0) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal,
> EfiGoneVirtual ());
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

On 05/24/17 18:52, Kinney, Michael D wrote:
> Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Commit b9036ebee9dd.

Thanks!
Laszlo

>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
>> Sent: Thursday, May 18, 2017 8:04 AM
>> To: edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>
>> Subject: [edk2] [PATCH 7/8] QuarkPlatformPkg/SpiFvbServices: correct NumOfLba vararg
>> type in EraseBlocks()
>>
>> According to the PI spec, Volume 3,
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
>>
>>> The variable argument list is a list of tuples. Each tuple describes a
>>> range of LBAs to erase and consists of the following:
>>> * An EFI_LBA that indicates the starting LBA
>>> * A UINTN that indicates the number of blocks to erase
>>
>> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
>> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
>>
>> In this driver, the NumOfLba local variable is defined with type UINTN,
>> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Kelly Steele <kelly.steele@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     unbuilt, untested
>>
>>  QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
>> b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
>> index dbb5512f386e..0e7a7b79a1cf 100644
>> --- a/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
>> +++ b/QuarkPlatformPkg/Platform/SpiFvbServices/FwBlockService.c
>> @@ -1211,7 +1211,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      //
>>      // Check input parameters
>> @@ -1235,7 +1235,7 @@ Returns:
>>        break;
>>      }
>>
>> -    NumOfLba = VA_ARG (args, UINT32);
>> +    NumOfLba = VA_ARG (args, UINTN);
>>
>>      while (NumOfLba > 0) {
>>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba, mFvbModuleGlobal,
>> EfiGoneVirtual ());
>> --
>> 2.9.3
>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

[PATCH 8/8] Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: David Wei <david.wei@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Mang Guo <mang.guo@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    unbuilt, untested

 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c | 4 ++--
 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
index b0013f918368..89b941042420 100644
--- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
+++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
@@ -793,7 +793,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters.
@@ -817,7 +817,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while ( NumOfLba > 0 ) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba);
diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
index 20aaf8720f72..22a4bdcdd492 100644
--- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
+++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
@@ -725,7 +725,7 @@ FvbEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (Marker, UINT32);
+    NumOfLba = VA_ARG (Marker, UINTN);
     if (NumOfLba == 0) {
       return EFI_INVALID_PARAMETER;
     }
@@ -742,7 +742,7 @@ FvbEraseBlocks (
     if (StartingLba == EFI_LBA_LIST_TERMINATOR ) {
       break;
     }
-    NumOfLba = VA_ARG (Marker, UINT32);
+    NumOfLba = VA_ARG (Marker, UINTN);
     Status = EraseBlock (This, StartingLba, NumOfLba);
     if (EFI_ERROR (Status)) {
       break;
-- 
2.9.3

Re: [PATCH 8/8] Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Wei, David]

Reviewed-by: zwei4 <david.wei@intel.com> 


Thanks,
David  Wei                                 

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Thursday, May 18, 2017 11:04 PM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Wei, David <david.wei@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Guo, Mang <mang.guo@intel.com>
Subject: [PATCH 8/8] Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

According to the PI spec, Volume 3,
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():

> The variable argument list is a list of tuples. Each tuple describes a
> range of LBAs to erase and consists of the following:
> * An EFI_LBA that indicates the starting LBA
> * A UINTN that indicates the number of blocks to erase

(NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)

In this driver, the NumOfLba local variable is defined with type UINTN,
but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.

Cc: David Wei <david.wei@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Mang Guo <mang.guo@intel.com>
Reported-by: Jordan Justen <jordan.l.justen@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    unbuilt, untested

 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c | 4 ++--
 Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
index b0013f918368..89b941042420 100644
--- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
+++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
@@ -793,7 +793,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     //
     // Check input parameters.
@@ -817,7 +817,7 @@ FvbProtocolEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (args, UINT32);
+    NumOfLba = VA_ARG (args, UINTN);
 
     while ( NumOfLba > 0 ) {
       Status = FvbEraseBlock (FvbDevice->Instance, StartingLba);
diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
index 20aaf8720f72..22a4bdcdd492 100644
--- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
+++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
@@ -725,7 +725,7 @@ FvbEraseBlocks (
       break;
     }
 
-    NumOfLba = VA_ARG (Marker, UINT32);
+    NumOfLba = VA_ARG (Marker, UINTN);
     if (NumOfLba == 0) {
       return EFI_INVALID_PARAMETER;
     }
@@ -742,7 +742,7 @@ FvbEraseBlocks (
     if (StartingLba == EFI_LBA_LIST_TERMINATOR ) {
       break;
     }
-    NumOfLba = VA_ARG (Marker, UINT32);
+    NumOfLba = VA_ARG (Marker, UINTN);
     Status = EraseBlock (This, StartingLba, NumOfLba);
     if (EFI_ERROR (Status)) {
       break;
-- 
2.9.3

Re: [PATCH 8/8] Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()

[Laszlo Ersek]

David,

On 05/19/17 03:49, Wei, David wrote:
> Reviewed-by: zwei4 <david.wei@intel.com> 
> 
> 
> Thanks,
> David  Wei                                 

thanks for committing & pushing this patch on my behalf -- I've been
waiting for the rest of the reviews to trickle in. I figured I'd push
the DuetPkg, Nt32Pkg, QuarkPlatformPkg and Vlv2TbltDevicePkg patches in
one go. The first two of these remain unreviewed (by the package
maintainer), which is why I haven't pushed the QuarkPlatformPkg and
Vlv2TbltDevicePkg patches either.

Thanks,
Laszlo

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com] 
> Sent: Thursday, May 18, 2017 11:04 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Wei, David <david.wei@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Guo, Mang <mang.guo@intel.com>
> Subject: [PATCH 8/8] Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks()
> 
> According to the PI spec, Volume 3,
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.EraseBlocks():
> 
>> The variable argument list is a list of tuples. Each tuple describes a
>> range of LBAs to erase and consists of the following:
>> * An EFI_LBA that indicates the starting LBA
>> * A UINTN that indicates the number of blocks to erase
> 
> (NB, in edk2, EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL is a typedef to
> EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL.)
> 
> In this driver, the NumOfLba local variable is defined with type UINTN,
> but the TYPE argument passed to VA_ARG() is UINT32. Fix the mismatch.
> 
> Cc: David Wei <david.wei@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Mang Guo <mang.guo@intel.com>
> Reported-by: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     unbuilt, untested
> 
>  Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c | 4 ++--
>  Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
> index b0013f918368..89b941042420 100644
> --- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
> +++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c
> @@ -793,7 +793,7 @@ FvbProtocolEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      //
>      // Check input parameters.
> @@ -817,7 +817,7 @@ FvbProtocolEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (args, UINT32);
> +    NumOfLba = VA_ARG (args, UINTN);
>  
>      while ( NumOfLba > 0 ) {
>        Status = FvbEraseBlock (FvbDevice->Instance, StartingLba);
> diff --git a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
> index 20aaf8720f72..22a4bdcdd492 100644
> --- a/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
> +++ b/Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbSmmDxe.c
> @@ -725,7 +725,7 @@ FvbEraseBlocks (
>        break;
>      }
>  
> -    NumOfLba = VA_ARG (Marker, UINT32);
> +    NumOfLba = VA_ARG (Marker, UINTN);
>      if (NumOfLba == 0) {
>        return EFI_INVALID_PARAMETER;
>      }
> @@ -742,7 +742,7 @@ FvbEraseBlocks (
>      if (StartingLba == EFI_LBA_LIST_TERMINATOR ) {
>        break;
>      }
> -    NumOfLba = VA_ARG (Marker, UINT32);
> +    NumOfLba = VA_ARG (Marker, UINTN);
>      Status = EraseBlock (This, StartingLba, NumOfLba);
>      if (EFI_ERROR (Status)) {
>        break;
>