In edk2, the division and shifting of 64-bit values are forbidden with
C-language operators, because the compiler may generate intrinsic calls
for them.
For example, clang-3.8 emits a call to "__umoddi3" for
UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32,
which then fails to link.
UDF_LOGICAL_SECTOR_SIZE has type UINT64, while
EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator
with a DivU64x32Remainder() call.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index c1d44809bfd2..c491ef25f47e 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -234,10 +234,11 @@ PartitionInstallUdfChildHandles (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
IN EFI_BLOCK_IO2_PROTOCOL *BlockIo2,
IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
)
{
+ UINT32 RemainderByMediaBlockSize;
EFI_STATUS Status;
EFI_BLOCK_IO_MEDIA *Media;
EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
EFI_GUID *VendorDefinedGuid;
EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
@@ -246,11 +247,16 @@ PartitionInstallUdfChildHandles (
Media = BlockIo->Media;
//
// Check if UDF logical block size is multiple of underlying device block size
//
- if ((UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize) != 0 ||
+ DivU64x32Remainder (
+ UDF_LOGICAL_SECTOR_SIZE, // Dividend
+ Media->BlockSize, // Divisor
+ &RemainderByMediaBlockSize // Remainder
+ );
+ if (RemainderByMediaBlockSize != 0 ||
Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE) {
return EFI_NOT_FOUND;
}
DevicePathNode = DevicePath;
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi Laszlo,
When do you plan to push this patch? IA32 build is blocked for this issue now.
Thanks,
Dandan
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Sunday, September 10, 2017 8:13 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
In edk2, the division and shifting of 64-bit values are forbidden with C-language operators, because the compiler may generate intrinsic calls for them.
For example, clang-3.8 emits a call to "__umoddi3" for
UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32, which then fails to link.
UDF_LOGICAL_SECTOR_SIZE has type UINT64, while EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator with a DivU64x32Remainder() call.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index c1d44809bfd2..c491ef25f47e 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -234,10 +234,11 @@ PartitionInstallUdfChildHandles (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
IN EFI_BLOCK_IO2_PROTOCOL *BlockIo2,
IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
)
{
+ UINT32 RemainderByMediaBlockSize;
EFI_STATUS Status;
EFI_BLOCK_IO_MEDIA *Media;
EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
EFI_GUID *VendorDefinedGuid;
EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
@@ -246,11 +247,16 @@ PartitionInstallUdfChildHandles (
Media = BlockIo->Media;
//
// Check if UDF logical block size is multiple of underlying device block size
//
- if ((UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize) != 0 ||
+ DivU64x32Remainder (
+ UDF_LOGICAL_SECTOR_SIZE, // Dividend
+ Media->BlockSize, // Divisor
+ &RemainderByMediaBlockSize // Remainder
+ );
+ if (RemainderByMediaBlockSize != 0 ||
Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE) {
return EFI_NOT_FOUND;
}
DevicePathNode = DevicePath;
--
2.14.1.3.gb7cf6e02401b
_______________________________________________
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
On 09/12/17 07:41, Bi, Dandan wrote:
> Hi Laszlo,
>
> When do you plan to push this patch? IA32 build is blocked for this issue now.
I was ready to push the series yesterday; I just hoped I'd get review
feedback from MdeModulePkg maintainers as well, and/or from Ray, in one
or two days.
These are strongly localized changes that require no knowledge of the
UDF driver. (I don't have that knowledge myself, to begin with.) At
least an Acked-by would be nice.
If someone from Intel tells me I can push this with the R-b's that are
currently on the list, I'm totally game.
Thanks!
Laszlo
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
>
> In edk2, the division and shifting of 64-bit values are forbidden with C-language operators, because the compiler may generate intrinsic calls for them.
>
> For example, clang-3.8 emits a call to "__umoddi3" for
>
> UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
>
> in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32, which then fails to link.
>
> UDF_LOGICAL_SECTOR_SIZE has type UINT64, while EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator with a DivU64x32Remainder() call.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> index c1d44809bfd2..c491ef25f47e 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> @@ -234,10 +234,11 @@ PartitionInstallUdfChildHandles (
> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> IN EFI_BLOCK_IO2_PROTOCOL *BlockIo2,
> IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
> )
> {
> + UINT32 RemainderByMediaBlockSize;
> EFI_STATUS Status;
> EFI_BLOCK_IO_MEDIA *Media;
> EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
> EFI_GUID *VendorDefinedGuid;
> EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> @@ -246,11 +247,16 @@ PartitionInstallUdfChildHandles (
> Media = BlockIo->Media;
>
> //
> // Check if UDF logical block size is multiple of underlying device block size
> //
> - if ((UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize) != 0 ||
> + DivU64x32Remainder (
> + UDF_LOGICAL_SECTOR_SIZE, // Dividend
> + Media->BlockSize, // Divisor
> + &RemainderByMediaBlockSize // Remainder
> + );
> + if (RemainderByMediaBlockSize != 0 ||
> Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE) {
> return EFI_NOT_FOUND;
> }
>
> DevicePathNode = DevicePath;
> --
> 2.14.1.3.gb7cf6e02401b
>
>
> _______________________________________________
> 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
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Thanks/Ray
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, September 12, 2017 3:59 PM
> To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel-01 <edk2-
> devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide
> 64-bit values with C operators
>
> On 09/12/17 07:41, Bi, Dandan wrote:
> > Hi Laszlo,
> >
> > When do you plan to push this patch? IA32 build is blocked for this issue
> now.
>
> I was ready to push the series yesterday; I just hoped I'd get review
> feedback from MdeModulePkg maintainers as well, and/or from Ray, in one
> or two days.
>
> These are strongly localized changes that require no knowledge of the UDF
> driver. (I don't have that knowledge myself, to begin with.) At least an
> Acked-by would be nice.
>
> If someone from Intel tells me I can push this with the R-b's that are currently
> on the list, I'm totally game.
>
> Thanks!
> Laszlo
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Laszlo Ersek
> > Sent: Sunday, September 10, 2017 8:13 AM
> > To: edk2-devel-01 <edk2-devel@lists.01.org>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>
> > Subject: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide
> > 64-bit values with C operators
> >
> > In edk2, the division and shifting of 64-bit values are forbidden with C-
> language operators, because the compiler may generate intrinsic calls for
> them.
> >
> > For example, clang-3.8 emits a call to "__umoddi3" for
> >
> > UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
> >
> > in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32, which
> then fails to link.
> >
> > UDF_LOGICAL_SECTOR_SIZE has type UINT64, while
> EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator
> with a DivU64x32Remainder() call.
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Paulo Alcantara <pcacjr@zytor.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > ---
> > MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> > b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> > index c1d44809bfd2..c491ef25f47e 100644
> > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> > @@ -234,10 +234,11 @@ PartitionInstallUdfChildHandles (
> > IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> > IN EFI_BLOCK_IO2_PROTOCOL *BlockIo2,
> > IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
> > )
> > {
> > + UINT32 RemainderByMediaBlockSize;
> > EFI_STATUS Status;
> > EFI_BLOCK_IO_MEDIA *Media;
> > EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
> > EFI_GUID *VendorDefinedGuid;
> > EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> > @@ -246,11 +247,16 @@ PartitionInstallUdfChildHandles (
> > Media = BlockIo->Media;
> >
> > //
> > // Check if UDF logical block size is multiple of underlying device block size
> > //
> > - if ((UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize) != 0 ||
> > + DivU64x32Remainder (
> > + UDF_LOGICAL_SECTOR_SIZE, // Dividend
> > + Media->BlockSize, // Divisor
> > + &RemainderByMediaBlockSize // Remainder
> > + );
> > + if (RemainderByMediaBlockSize != 0 ||
> > Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE) {
> > return EFI_NOT_FOUND;
> > }
> >
> > DevicePathNode = DevicePath;
> > --
> > 2.14.1.3.gb7cf6e02401b
> >
> >
> > _______________________________________________
> > 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
Reviewed-by: Star Zeng <star.zeng@intel.com>
I am not sure whether the other patches in this series has been reviewed or not.
Since this patch is fixing build break, I think we can have this patch pushed first after reviewed. And really appreciate that. :)
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Tuesday, September 12, 2017 3:59 PM
To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
On 09/12/17 07:41, Bi, Dandan wrote:
> Hi Laszlo,
>
> When do you plan to push this patch? IA32 build is blocked for this issue now.
I was ready to push the series yesterday; I just hoped I'd get review feedback from MdeModulePkg maintainers as well, and/or from Ray, in one or two days.
These are strongly localized changes that require no knowledge of the UDF driver. (I don't have that knowledge myself, to begin with.) At least an Acked-by would be nice.
If someone from Intel tells me I can push this with the R-b's that are currently on the list, I'm totally game.
Thanks!
Laszlo
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 4/5] MdeModulePkg/PartitionDxe: don't divide
> 64-bit values with C operators
>
> In edk2, the division and shifting of 64-bit values are forbidden with C-language operators, because the compiler may generate intrinsic calls for them.
>
> For example, clang-3.8 emits a call to "__umoddi3" for
>
> UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize
>
> in PartitionInstallUdfChildHandles(), if PartitionDxe is built for IA32, which then fails to link.
>
> UDF_LOGICAL_SECTOR_SIZE has type UINT64, while EFI_BLOCK_IO_MEDIA.BlockSize has type UINT32(). Replace the % operator with a DivU64x32Remainder() call.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> index c1d44809bfd2..c491ef25f47e 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> @@ -234,10 +234,11 @@ PartitionInstallUdfChildHandles (
> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> IN EFI_BLOCK_IO2_PROTOCOL *BlockIo2,
> IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
> )
> {
> + UINT32 RemainderByMediaBlockSize;
> EFI_STATUS Status;
> EFI_BLOCK_IO_MEDIA *Media;
> EFI_DEVICE_PATH_PROTOCOL *DevicePathNode;
> EFI_GUID *VendorDefinedGuid;
> EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> @@ -246,11 +247,16 @@ PartitionInstallUdfChildHandles (
> Media = BlockIo->Media;
>
> //
> // Check if UDF logical block size is multiple of underlying device block size
> //
> - if ((UDF_LOGICAL_SECTOR_SIZE % Media->BlockSize) != 0 ||
> + DivU64x32Remainder (
> + UDF_LOGICAL_SECTOR_SIZE, // Dividend
> + Media->BlockSize, // Divisor
> + &RemainderByMediaBlockSize // Remainder
> + );
> + if (RemainderByMediaBlockSize != 0 ||
> Media->BlockSize > UDF_LOGICAL_SECTOR_SIZE) {
> return EFI_NOT_FOUND;
> }
>
> DevicePathNode = DevicePath;
> --
> 2.14.1.3.gb7cf6e02401b
>
>
> _______________________________________________
> 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
© 2016 - 2024 Red Hat, Inc.