diff mbox

[edk2,00/19] OvmfPkg, ArmVirtPkg: leave deprecated interfaces behind

Message ID CAKv+Gu-vv__veC7qbHbLvi7_tOVdd_d+YEOxtrSWAFtXoE5F6g@mail.gmail.com
State New
Headers show

Commit Message

Ard Biesheuvel Oct. 24, 2016, 8:04 a.m. UTC 
On 21 October 2016 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote:
> This series intends to solve the following BZs:

>

>   <https://bugzilla.tianocore.org/show_bug.cgi?id=166> -- OvmfPkg: Add

>   the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC

>   files

>

>   <https://bugzilla.tianocore.org/show_bug.cgi?id=165> -- ArmVirtPkg:

>   Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package

>   DSC files

>

> Public branch:

> <https://github.com/lersek/edk2/commits/no_deprec_ovmf_bz166_armvirt_bz165>.

>

> The DISABLE_NEW_DEPRECATED_INTERFACES feature test macro disables the

> following MdePkg library class APIs:

>

>   BaseLib:

>   - StrCpy, StrnCpy, StrCat, StrnCat, UnicodeStrToAsciiStr,

>   - AsciiStrCpy, AsciiStrnCpy, AsciiStrCat, AsciiStrnCat,

>     AsciiStrToUnicodeStr.

>

>   PcdLib:

>   - PcdSet8, PcdSet16, PcdSet32, PcdSet64, PcdSetPtr, PcdSetBool,

>   - PcdSetEx8, PcdSetEx16, PcdSetEx32, PcdSetEx64, PcdSetExPtr,

>     PcdSetExBool.

>

>   UefiLib:

>   - GetVariable, GetEfiGlobalVariable.

>

> The series gradually weans the OvmfPkg and ArmVirtPkg modules off these.

> For the 32-bit ARM builds of ArmVirtPkg platforms, I had to dip my toes

> into ArmPkg a little bit, due to dependencies.

>

> I couldn't build-test some changes (for example, the only compiler

> toolchains I have access to at the moment are GCC48 for Ia32/X64, and

> GCC5 for ARM/AARCH64). Some changes I could build, but not functionally

> test (Xen en bloc, 32-bit ARM, RAM-emulated variables in OVMF, -bios

> flag). For all of these, I liberally sprinkled the patches with Cc's and

> Notes sections, asking for help. I did make an honest effort to build

> the ArmVirt and OVMF platforms in as many configurations (-D ...) as I

> could think of, perusing the various !if directives in the DSC files.

>


I tried ArmVirtQemu.dsc in DEBUG mode with RVCTLINUX, and it built fine*

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT


Thanks,
Ard.


* In general, RVCT tends to fall over quite regularly due to its
finicky diagnostics, so I did have to apply this patch

"""
+
"""

but these changes are entirely unrelated to the series, and I will
follow up with some patches to fix this.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Oct. 24, 2016, 11:50 a.m. UTC | #1 
On 10/24/16 10:04, Ard Biesheuvel wrote:
> On 21 October 2016 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote:

>> This series intends to solve the following BZs:

>>

>>   <https://bugzilla.tianocore.org/show_bug.cgi?id=166> -- OvmfPkg: Add

>>   the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC

>>   files

>>

>>   <https://bugzilla.tianocore.org/show_bug.cgi?id=165> -- ArmVirtPkg:

>>   Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package

>>   DSC files

>>

>> Public branch:

>> <https://github.com/lersek/edk2/commits/no_deprec_ovmf_bz166_armvirt_bz165>.

>>

>> The DISABLE_NEW_DEPRECATED_INTERFACES feature test macro disables the

>> following MdePkg library class APIs:

>>

>>   BaseLib:

>>   - StrCpy, StrnCpy, StrCat, StrnCat, UnicodeStrToAsciiStr,

>>   - AsciiStrCpy, AsciiStrnCpy, AsciiStrCat, AsciiStrnCat,

>>     AsciiStrToUnicodeStr.

>>

>>   PcdLib:

>>   - PcdSet8, PcdSet16, PcdSet32, PcdSet64, PcdSetPtr, PcdSetBool,

>>   - PcdSetEx8, PcdSetEx16, PcdSetEx32, PcdSetEx64, PcdSetExPtr,

>>     PcdSetExBool.

>>

>>   UefiLib:

>>   - GetVariable, GetEfiGlobalVariable.

>>

>> The series gradually weans the OvmfPkg and ArmVirtPkg modules off these.

>> For the 32-bit ARM builds of ArmVirtPkg platforms, I had to dip my toes

>> into ArmPkg a little bit, due to dependencies.

>>

>> I couldn't build-test some changes (for example, the only compiler

>> toolchains I have access to at the moment are GCC48 for Ia32/X64, and

>> GCC5 for ARM/AARCH64). Some changes I could build, but not functionally

>> test (Xen en bloc, 32-bit ARM, RAM-emulated variables in OVMF, -bios

>> flag). For all of these, I liberally sprinkled the patches with Cc's and

>> Notes sections, asking for help. I did make an honest effort to build

>> the ArmVirt and OVMF platforms in as many configurations (-D ...) as I

>> could think of, perusing the various !if directives in the DSC files.

>>

> 

> I tried ArmVirtQemu.dsc in DEBUG mode with RVCTLINUX, and it built fine*

> 

> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT


Thank you!

Then,

> * In general, RVCT tends to fall over quite regularly due to its

> finicky diagnostics, so I did have to apply this patch

> 

> """

> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> index ca61ac5e1983..1098d9501cc7 100644

> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> @@ -891,7 +891,7 @@ NorFlashRead (

>    SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);

> 

>    // Readout the data

> -  AlignedCopyMem (Buffer, (VOID *)StartAddress + Offset, BufferSizeInBytes);

> +  AlignedCopyMem (Buffer, (VOID *)(StartAddress + Offset), BufferSizeInBytes);

> 

>    return EFI_SUCCESS;

>  }


RVCT is correct here; standard C does not permit arithmetic on
pointers-to-void. From C99 for example,

    6.5.6 Additive operators
    ...
    Constraints

  2 For addition, either both operands shall have arithmetic type, or
    one operand shall be a pointer to an object type and the other
    shall have integer type. [...]

"Pointer to void" is not "pointer to object type".

Although in

    6.2.5 Types

we have

  27 A pointer to void shall have the same representation and alignment
     requirements as a pointer to a character type. [...]

but also:

  19 The void type comprises an empty set of values; it is an
     incomplete type that cannot be completed.

So, not an object type.

Can you submit this patch per se? It's a valid and justified change.

> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> index 812dafd065b2..0ef7b8d81bbc 100644

> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> @@ -71,3 +71,7 @@ [Depex]

>    # NorFlashDxe must be loaded before VariableRuntimeDxe in case

> empty flash needs populating with default values

>    #

>    BEFORE gVariableRuntimeDxeFileGuid

> +

> +[BuildOptions]

> +  RVCT:*_*_*_CC_FLAGS = --diag_suppress=6314

> +

> """


No input on this though :)

> but these changes are entirely unrelated to the series, and I will

> follow up with some patches to fix this.

> 


Yes, please.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Oct. 25, 2016, 8:26 a.m. UTC | #2 
On 10/24/16 10:04, Ard Biesheuvel wrote:
> On 21 October 2016 at 22:27, Laszlo Ersek <lersek@redhat.com> wrote:

>> This series intends to solve the following BZs:

>>

>>   <https://bugzilla.tianocore.org/show_bug.cgi?id=166> -- OvmfPkg: Add

>>   the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC

>>   files

>>

>>   <https://bugzilla.tianocore.org/show_bug.cgi?id=165> -- ArmVirtPkg:

>>   Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package

>>   DSC files

>>

>> Public branch:

>> <https://github.com/lersek/edk2/commits/no_deprec_ovmf_bz166_armvirt_bz165>.

>>

>> The DISABLE_NEW_DEPRECATED_INTERFACES feature test macro disables the

>> following MdePkg library class APIs:

>>

>>   BaseLib:

>>   - StrCpy, StrnCpy, StrCat, StrnCat, UnicodeStrToAsciiStr,

>>   - AsciiStrCpy, AsciiStrnCpy, AsciiStrCat, AsciiStrnCat,

>>     AsciiStrToUnicodeStr.

>>

>>   PcdLib:

>>   - PcdSet8, PcdSet16, PcdSet32, PcdSet64, PcdSetPtr, PcdSetBool,

>>   - PcdSetEx8, PcdSetEx16, PcdSetEx32, PcdSetEx64, PcdSetExPtr,

>>     PcdSetExBool.

>>

>>   UefiLib:

>>   - GetVariable, GetEfiGlobalVariable.

>>

>> The series gradually weans the OvmfPkg and ArmVirtPkg modules off these.

>> For the 32-bit ARM builds of ArmVirtPkg platforms, I had to dip my toes

>> into ArmPkg a little bit, due to dependencies.

>>

>> I couldn't build-test some changes (for example, the only compiler

>> toolchains I have access to at the moment are GCC48 for Ia32/X64, and

>> GCC5 for ARM/AARCH64). Some changes I could build, but not functionally

>> test (Xen en bloc, 32-bit ARM, RAM-emulated variables in OVMF, -bios

>> flag). For all of these, I liberally sprinkled the patches with Cc's and

>> Notes sections, asking for help. I did make an honest effort to build

>> the ArmVirt and OVMF platforms in as many configurations (-D ...) as I

>> could think of, perusing the various !if directives in the DSC files.

>>

> 

> I tried ArmVirtQemu.dsc in DEBUG mode with RVCTLINUX, and it built fine*

> 

> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT


Okay, I'll add this tag to the ArmVirtPkg patches.

Thanks,
Laszlo


> 

> Thanks,

> Ard.

> 

> 

> * In general, RVCT tends to fall over quite regularly due to its

> finicky diagnostics, so I did have to apply this patch

> 

> """

> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> index ca61ac5e1983..1098d9501cc7 100644

> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c

> @@ -891,7 +891,7 @@ NorFlashRead (

>    SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);

> 

>    // Readout the data

> -  AlignedCopyMem (Buffer, (VOID *)StartAddress + Offset, BufferSizeInBytes);

> +  AlignedCopyMem (Buffer, (VOID *)(StartAddress + Offset), BufferSizeInBytes);

> 

>    return EFI_SUCCESS;

>  }

> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> index 812dafd065b2..0ef7b8d81bbc 100644

> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf

> @@ -71,3 +71,7 @@ [Depex]

>    # NorFlashDxe must be loaded before VariableRuntimeDxe in case

> empty flash needs populating with default values

>    #

>    BEFORE gVariableRuntimeDxeFileGuid

> +

> +[BuildOptions]

> +  RVCT:*_*_*_CC_FLAGS = --diag_suppress=6314

> +

> """

> 

> but these changes are entirely unrelated to the series, and I will

> follow up with some patches to fix this.

> _______________________________________________

> 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
Laszlo Ersek Oct. 25, 2016, 8:45 a.m. UTC | #3 
On 10/25/16 10:26, Laszlo Ersek wrote:
> On 10/24/16 10:04, Ard Biesheuvel wrote:


>> I tried ArmVirtQemu.dsc in DEBUG mode with RVCTLINUX, and it built fine*

>>

>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT

> 

> Okay, I'll add this tag to the ArmVirtPkg patches.


Actually, I'm adding it to the following non-ArmVirtPkg patches as well:

- MdePkg/DebugLib.h: add ASSERT_RETURN_ERROR()
- OvmfPkg/SmbiosVersionLib: eliminate unchecked PcdSetXX() calls
- OvmfPkg/PlatformDxe: eliminate unchecked PcdSetXX() calls

because, according to the ArmVirtQemu build report file, these modules
are pulled into ArmVirtQemu.

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 25, 2016, 8:49 a.m. UTC | #4 
On 25 October 2016 at 09:45, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/25/16 10:26, Laszlo Ersek wrote:

>> On 10/24/16 10:04, Ard Biesheuvel wrote:

>

>>> I tried ArmVirtQemu.dsc in DEBUG mode with RVCTLINUX, and it built fine*

>>>

>>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> # RVCT

>>

>> Okay, I'll add this tag to the ArmVirtPkg patches.

>

> Actually, I'm adding it to the following non-ArmVirtPkg patches as well:

>

> - MdePkg/DebugLib.h: add ASSERT_RETURN_ERROR()

> - OvmfPkg/SmbiosVersionLib: eliminate unchecked PcdSetXX() calls

> - OvmfPkg/PlatformDxe: eliminate unchecked PcdSetXX() calls

>

> because, according to the ArmVirtQemu build report file, these modules

> are pulled into ArmVirtQemu.

>


That's fine
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index ca61ac5e1983..1098d9501cc7 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -891,7 +891,7 @@  NorFlashRead (
   SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);

   // Readout the data
-  AlignedCopyMem (Buffer, (VOID *)StartAddress + Offset, BufferSizeInBytes);
+  AlignedCopyMem (Buffer, (VOID *)(StartAddress + Offset), BufferSizeInBytes);

   return EFI_SUCCESS;
 }
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index 812dafd065b2..0ef7b8d81bbc 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -71,3 +71,7 @@  [Depex]
   # NorFlashDxe must be loaded before VariableRuntimeDxe in case
empty flash needs populating with default values
   #
   BEFORE gVariableRuntimeDxeFileGuid
+
+[BuildOptions]
+  RVCT:*_*_*_CC_FLAGS = --diag_suppress=6314