[edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg

Michael D Kinney michael.d.kinney at intel.com
Thu Aug 5 18:33:19 UTC 2021


Hi Pedro,

1) Ext4Pkg/Ext4Dxe/Ext4Dxe.inf:
  
  * To be consistent with other drivers, BASE_NAME should be changed from Ext4 to Ext4Dxe.
  * For proper dependency checking in incremental builds, please add the .h files to the [Sources] section

      Ext4Disk.h
      Ext4Dxe.h

2) There are a number of code style issues that need to be addressed.  Can you fix those for V2?

3) I did a quick pass to find the IA32 NOOPT VS2019 issues.  With the following changes, I can get it to build.  Do not know if I introduced any functional changes by mistake.

diff --git a/Features/Ext4Pkg/Ext4Dxe/BlockGroup.c b/Features/Ext4Pkg/Ext4Dxe/BlockGroup.c
index 10a82d40a0..f2db93f02c 100644
--- a/Features/Ext4Pkg/Ext4Dxe/BlockGroup.c
+++ b/Features/Ext4Pkg/Ext4Dxe/BlockGroup.c
@@ -61,7 +61,7 @@ Ext4ReadInode (
              Partition,
              Inode,
              Partition->InodeSize,
-             Ext4BlockToByteOffset (Partition, InodeTableStart) + InodeOffset * Partition->InodeSize
+             Ext4BlockToByteOffset (Partition, InodeTableStart) + MultU64x32 (InodeOffset, Partition->InodeSize)
              );
 
   if (EFI_ERROR (Status)) {
diff --git a/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c b/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c
index 1cafdd64cd..65109809c0 100644
--- a/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c
+++ b/Features/Ext4Pkg/Ext4Dxe/DiskUtil.c
@@ -45,7 +45,7 @@ Ext4ReadBlocks (
   IN EXT4_BLOCK_NR BlockNumber
   )
 {
-  return Ext4ReadDiskIo (Partition, Buffer, NumberBlocks * Partition->BlockSize, BlockNumber * Partition->BlockSize);
+  return Ext4ReadDiskIo (Partition, Buffer, NumberBlocks * Partition->BlockSize, MultU64x32 (BlockNumber, Partition->BlockSize));
 }
 
 /**
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index d790e70be1..8aa584df14 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -445,6 +445,6 @@ typedef struct {
 typedef UINT64  EXT4_BLOCK_NR;
 typedef UINT32  EXT4_INO_NR;
 
-#define EXT4_INODE_SIZE(ino)  (((UINT64)ino->i_size_hi << 32) | ino->i_size_lo)
+#define EXT4_INODE_SIZE(ino)  (LShiftU64 (ino->i_size_hi, 32) | ino->i_size_lo)
 
 #endif
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index f6875c919e..a055a139e1 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -244,7 +244,7 @@ Ext4MakeBlockNumberFromHalfs (
   )
 {
   // High might have garbage if it's not a 64 bit filesystem
-  return Ext4Is64Bit (Partition) ? Low | ((UINT64)High << 32) : Low;
+  return Ext4Is64Bit (Partition) ? (Low | LShiftU64 (High, 32)) : Low;
 }
 
 /**
@@ -297,7 +297,7 @@ Ext4BlockToByteOffset (
   IN EXT4_BLOCK_NR Block
   )
 {
-  return Partition->BlockSize * Block;
+  return MultU64x32 (Block, Partition->BlockSize);
 }
 
 /**
@@ -333,7 +333,7 @@ Ext4InodeSize (
   CONST EXT4_INODE *Inode
   )
 {
-  return ((UINT64)Inode->i_size_hi << 32) | Inode->i_size_lo;
+  return (LShiftU64 (Inode->i_size_hi, 32) | Inode->i_size_lo);
 }
 
 /**
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
index 102b12d613..fc0185285e 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
@@ -111,6 +111,8 @@ [Sources]
   Collation.c
   Crc32c.c
   Crc16.c
+  Ext4Disk.h
+  Ext4Dxe.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/Features/Ext4Pkg/Ext4Dxe/Extents.c b/Features/Ext4Pkg/Ext4Dxe/Extents.c
index db4bf5aa3f..8c9b4a4c75 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Extents.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Extents.c
@@ -210,7 +210,7 @@ Ext4ExtentIdxLeafBlock (
   IN EXT4_EXTENT_INDEX *Index
   )
 {
-  return ((UINT64)Index->ei_leaf_hi << 32) | Index->ei_leaf_lo;
+  return LShiftU64(Index->ei_leaf_hi, 32) | Index->ei_leaf_lo;
 }
 
 STATIC UINTN  GetExtentRequests  = 0;
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index 10dda64b16..71d36d1990 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -487,8 +487,8 @@ Ext4GetFilesystemInfo (
   Info->BlockSize = Part->BlockSize;
   Info->Size       = NeededLength;
   Info->ReadOnly   = Part->ReadOnly;
-  Info->VolumeSize = TotalBlocks * Part->BlockSize;
-  Info->FreeSpace  = FreeBlocks * Part->BlockSize;
+  Info->VolumeSize = MultU64x32 (TotalBlocks, Part->BlockSize);
+  Info->FreeSpace  = MultU64x32 (FreeBlocks, Part->BlockSize);
 
   if (VolumeName != NULL) {
     StrCpyS (Info->VolumeLabel, VolNameLength + 1, VolumeName);
diff --git a/Features/Ext4Pkg/Ext4Dxe/Inode.c b/Features/Ext4Pkg/Ext4Dxe/Inode.c
index 304bf0c4a9..2a9f534d7e 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Inode.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Inode.c
@@ -154,7 +154,7 @@ Ext4Read (
       UINT64  ExtentOffset;
       UINTN   ExtentMayRead;
 
-      ExtentStartBytes   = (((UINT64)Extent.ee_start_hi << 32) | Extent.ee_start_lo) * Partition->BlockSize;
+      ExtentStartBytes   = MultU64x32 (LShiftU64 (Extent.ee_start_hi, 32) | Extent.ee_start_lo, Partition->BlockSize);
       ExtentLengthBytes  = Extent.ee_len * Partition->BlockSize;
       ExtentLogicalBytes = (UINT64)Extent.ee_block * Partition->BlockSize;
       ExtentOffset  = CurrentSeek - ExtentLogicalBytes;
@@ -276,17 +276,17 @@ Ext4FilePhysicalSpace (
   Blocks   = File->Inode->i_blocks;
 
   if(HugeFile) {
-    Blocks |= ((UINT64)File->Inode->i_osd2.data_linux.l_i_blocks_high) << 32;
+    Blocks |= LShiftU64 (File->Inode->i_osd2.data_linux.l_i_blocks_high, 32);
 
     // If HUGE_FILE is enabled and EXT4_HUGE_FILE_FL is set in the inode's flags, each unit
     // in i_blocks corresponds to an actual filesystem block
     if(File->Inode->i_flags & EXT4_HUGE_FILE_FL) {
-      return Blocks * File->Partition->BlockSize;
+      return MultU64x32 (Blocks, File->Partition->BlockSize);
     }
   }
 
   // Else, each i_blocks unit corresponds to 512 bytes
-  return Blocks * 512;
+  return MultU64x32 (Blocks, 512);
 }
 
 // Copied from EmbeddedPkg at my mentor's request.
@@ -368,7 +368,7 @@ EpochToEfiTime (
     UINT32      Nanoseconds  = 0;                                \
                                                            \
     if (Ext4InodeHasField (Inode, Field ## _extra)) {          \
-      SecondsEpoch |= ((UINT64)(Inode->Field ## _extra & EXT4_EXTRA_TIMESTAMP_MASK)) << 32; \
+      SecondsEpoch |= LShiftU64 ((UINT64)(Inode->Field ## _extra & EXT4_EXTRA_TIMESTAMP_MASK), 32); \
       Nanoseconds   = Inode->Field ## _extra >> 2;                                            \
     }                                                                                       \
     EpochToEfiTime ((UINTN)SecondsEpoch, Time);                                                     \
diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
index 18d8295a1f..88d01b62a8 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
+++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
@@ -161,7 +161,7 @@ Ext4OpenSuperblock (
 
   DEBUG ((EFI_D_INFO, "Read only = %u\n", Partition->ReadOnly));
 
-  Partition->BlockSize = 1024 << Sb->s_log_block_size;
+  Partition->BlockSize = (UINT32)LShiftU64 (1024, Sb->s_log_block_size);
 
   // The size of a block group can also be calculated as 8 * Partition->BlockSize
   if(Sb->s_blocks_per_group != 8 * Partition->BlockSize) {
@@ -195,7 +195,7 @@ Ext4OpenSuperblock (
   }
 
   NrBlocks = (UINTN)DivU64x32Remainder (
-                              Partition->NumberBlockGroups * Partition->DescSize,
+                              MultU64x32 (Partition->NumberBlockGroups, Partition->DescSize),
                               Partition->BlockSize,
                               &NrBlocksRem
                               );
diff --git a/Features/Ext4Pkg/Ext4Pkg.dsc b/Features/Ext4Pkg/Ext4Pkg.dsc
index 62cb4e69cf..57f279a4d9 100644
--- a/Features/Ext4Pkg/Ext4Pkg.dsc
+++ b/Features/Ext4Pkg/Ext4Pkg.dsc
@@ -20,6 +20,8 @@ [Defines]
   BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
   SKUID_IDENTIFIER               = DEFAULT
 
+!include MdePkg/MdeLibs.dsc.inc
+
 [BuildOptions]
   *_*_*_CC_FLAGS                       = -D DISABLE_NEW_DEPRECATED_INTERFACES




Thanks,

Mike




> -----Original Message-----
> From: Pedro Falcato <pedro.falcato at gmail.com>
> Sent: Friday, July 30, 2021 9:17 AM
> To: devel at edk2.groups.io
> Cc: Pedro Falcato <pedro.falcato at gmail.com>; Leif Lindholm <leif at nuviainc.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>; Bret Barkelew <Bret.Barkelew at microsoft.com>
> Subject: [Patch 0/3] Ext4Pkg: Add Ext4Pkg
> 
> This patch-set adds Ext4Pkg, a package designed to hold various drivers and
> utilities related to the EXT4 filesystem.
> 
> Right now, it holds a single read-only UEFI EXT4 driver (Ext4Dxe), which consumes the
> DISK_IO, BLOCK_IO and DISK_IO2 protocols and produce EFI_FILE_PROTOCOL and
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL; this driver allows the mounting of EXT4 partitions and
> the reading of their contents.
> 
> Relevant RFC discussion, which includes a more in-depth walkthrough of EXT4 internals and
> driver limitations is available at https://edk2.groups.io/g/devel/topic/84368561.
> 
> Cc: Leif Lindholm <leif at nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
> 
> Pedro Falcato (3):
>   Ext4Pkg: Add Ext4Pkg.dec and Ext4Pkg.uni.
>   Ext4Pkg: Add Ext4Dxe driver.
>   Ext4Pkg: Add .DSC file.
> 
>  Features/Ext4Pkg/Ext4Dxe/BlockGroup.c | 208 ++++++
>  Features/Ext4Pkg/Ext4Dxe/Collation.c  | 157 +++++
>  Features/Ext4Pkg/Ext4Dxe/Crc16.c      |  75 ++
>  Features/Ext4Pkg/Ext4Dxe/Crc32c.c     |  84 +++
>  Features/Ext4Pkg/Ext4Dxe/Directory.c  | 492 ++++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/DiskUtil.c   |  83 +++
>  Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h   | 450 ++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c    | 454 +++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h    | 942 ++++++++++++++++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf  | 147 ++++
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.uni  |  15 +
>  Features/Ext4Pkg/Ext4Dxe/Extents.c    | 616 +++++++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/File.c       | 583 ++++++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/Inode.c      | 468 +++++++++++++
>  Features/Ext4Pkg/Ext4Dxe/Partition.c  | 120 ++++
>  Features/Ext4Pkg/Ext4Dxe/Superblock.c | 257 +++++++
>  Features/Ext4Pkg/Ext4Pkg.dec          |  17 +
>  Features/Ext4Pkg/Ext4Pkg.dsc          |  68 ++
>  Features/Ext4Pkg/Ext4Pkg.uni          |  14 +
>  19 files changed, 5250 insertions(+)
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/BlockGroup.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Collation.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Crc16.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Crc32c.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Directory.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/DiskUtil.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.uni
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Extents.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/File.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Inode.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Partition.c
>  create mode 100644 Features/Ext4Pkg/Ext4Dxe/Superblock.c
>  create mode 100644 Features/Ext4Pkg/Ext4Pkg.dec
>  create mode 100644 Features/Ext4Pkg/Ext4Pkg.dsc
>  create mode 100644 Features/Ext4Pkg/Ext4Pkg.uni
> 
> --
> 2.32.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78748): https://edk2.groups.io/g/devel/message/78748
Mute This Topic: https://groups.io/mt/84553674/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list