Reviewed-by: Star Zeng <[email protected]>

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:[email protected]] 
Sent: Tuesday, September 12, 2017 3:59 PM
To: Bi, Dandan <[email protected]>; edk2-devel-01 <[email protected]>
Cc: Ni, Ruiyu <[email protected]>; Dong, Eric <[email protected]>; Zeng, 
Star <[email protected]>; Ard Biesheuvel <[email protected]>
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:[email protected]] On Behalf Of 
> Laszlo Ersek
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 <[email protected]>
> Cc: Ni, Ruiyu <[email protected]>; Dong, Eric <[email protected]>; 
> Zeng, Star <[email protected]>; Ard Biesheuvel 
> <[email protected]>
> 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 <[email protected]>
> Cc: Eric Dong <[email protected]>
> Cc: Paulo Alcantara <[email protected]>
> Cc: Ruiyu Ni <[email protected]>
> Cc: Star Zeng <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <[email protected]>
> ---
>  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
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to