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

Pedro Falcato posted 3 patches 2 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20210730161641.7245-1-pedro.falcato@gmail.com
There is a newer version of this series
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
[edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Pedro Falcato 2 years, 8 months ago
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@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@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 (#78444): https://edk2.groups.io/g/devel/message/78444
Mute This Topic: https://groups.io/mt/84553674/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Michael D Kinney 2 years, 7 months ago
Hi Pedro,

When I run BaseTools/Scripts/Patchcheck.py on this patch series, I see the following issues:


Checking git commit: 5f0db07586
Ext4Pkg: Add Ext4Dxe driver.
The commit message format passed all checks.
Code format is not valid:
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:     DEBUG ((EFI_D_ERROR, "[ext4] Could not open root inode - status %x\n", Status));
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:     DEBUG ((EFI_D_ERROR, "[ext4] dirent size %lu too small (compared to %lu)\n", Dirent->rec_len, RequiredSize));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Ext4ReadDir offset %lu\n", Offset));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:       DEBUG ((EFI_D_INFO, "[ext4] Length read %lu, offset %lu\n", Len, Offset));
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:       DEBUG ((EFI_D_ERROR, "[ext4] Invalid dirent at offset %lu\n", Offset));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:     DEBUG ((EFI_D_INFO, "[ext4] dirent size %lu\n", Entry.rec_len));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Directory.c
   Line:       DEBUG ((EFI_D_INFO, "[ext4] Listing file %s\n", TempFile->FileName));
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
   Line:
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
   Line:   DEBUG ((EFI_D_INFO, "[Ext4] Binding to controller\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
   Line:   DEBUG ((EFI_D_INFO, "[Ext4] Controller supports DISK_IO\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
   Line:     DEBUG ((EFI_D_INFO, "[Ext4] Controller supports DISK_IO2\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
   Line:   DEBUG ((EFI_D_INFO, "Opening partition\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Error mounting %x\n", Status));
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.inf
   Line: #
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:     DEBUG ((EFI_D_ERROR, "[ext4] Invalid extent header depth %u\n", Header->eh_depth));
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:     DEBUG ((EFI_D_ERROR, "[ext4] Invalid extent header magic %x\n", Header->eh_magic));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Looking up extent for block %lu\n", LogicalBlock));
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:       DEBUG ((EFI_D_ERROR, "[ext4] Invalid extent checksum\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:   /* DEBUG((EFI_D_INFO, "[ext4] extent 1 %u extent 2 %u = %ld\n", Extent1->ee_block,
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Extents.c
   Line:   // DEBUG((EFI_D_INFO, "[ext4] comparing %u %u\n", Block, Extent->ee_block));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Ext4Open %s\n", FileName));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:     DEBUG ((EFI_D_INFO, "[ext4] Opening %s\n", PathSegment));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:   DEBUG ((EFI_D_INFO, "Open successful\n"));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:   DEBUG ((EFI_D_INFO, "Opened filename %s\n", Current->FileName));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Closed file %p (inode %lu)\n", File, File->InodeNum));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:     DEBUG ((EFI_D_INFO, "[ext4] ReadDir status %lx\n", Status));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/File.c
   Line:       DEBUG ((EFI_D_INFO, "[ext4] ReadDir retlen %lu\n", *BufferSize));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Inode.c
   Line:   // DEBUG((EFI_D_INFO, "Ext4Read[Offset %lu, Length %lu]\n", Offset, *Length));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Inode.c
   Line:       // DEBUG((EFI_D_INFO, "[ext4] may read %lu, remaining %lu\n", ExtentMayRead, RemainingRead));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Inode.c
   Line:       // DEBUG((EFI_D_INFO, "[ext4] Reading block %lu\n", (ExtentStartBytes + ExtentOffset) / Partition->BlockSize));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Inode.c
   Line:   // DEBUG((EFI_D_INFO, "File length %lu crc %x\n", BeenRead, CalculateCrc32(original, BeenRead)));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Inode.c
   Line:     DEBUG ((EFI_D_INFO, "[ext4] Inode %d csum %x vs %x\n", InodeNum, Csum, DiskCsum));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:     DEBUG ((EFI_D_INFO, "[Ext4] Unsupported %lx\n", Partition->FeaturesIncompat & ~gSupportedIncompatFeat));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:     DEBUG ((EFI_D_INFO, "[Ext4] Unsupported ro compat %x\n", UnsupportedRoCompat));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:   DEBUG ((EFI_D_INFO, "Read only = %u\n", Partition->ReadOnly));
 * EFI_D_ERROR was used, but DEBUG_ERROR is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:     DEBUG ((EFI_D_ERROR, "[ext4] Bad superblock checksum %lx\n", Ext4CalculateSuperblockChecksum (Partition, Sb)));
 * Trailing whitespace found
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:       DEBUG ((EFI_D_INFO, "[ext4] Block group descriptor %u has an invalid checksum\n", Index));
 * EFI_D_INFO was used, but DEBUG_INFO is now recommended
   File: Features/Ext4Pkg/Ext4Dxe/Superblock.c
   Line:   DEBUG ((EFI_D_INFO, "[ext4] Root File %p\n", Partition->Root));

Best regards,

Mike


> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Friday, July 30, 2021 9:17 AM
> To: devel@edk2.groups.io
> Cc: Pedro Falcato <pedro.falcato@gmail.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@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@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@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 (#78718): https://edk2.groups.io/g/devel/message/78718
Mute This Topic: https://groups.io/mt/84553674/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Michael D Kinney 2 years, 7 months ago
Hi Pedro,

There is a build failure when building the Ext4Pkg.dsc file for a missing RegisterFilerLib mapping.

The correct fix for this issue is to add the following !include statement to the DSC file after the
the [Defines] section.

!include MdePkg/MdeLibs.dsc.inc

With this one change, the X64 VS2019 builds pass.

There is an additional issue for IA32 VS2019 NOOPT builds.

build -a IA32 -n 5 -t VS2019 -p Features\Ext4Pkg\Ext4Pkg.dsc -b NOOPT

Ext4.lib(DiskUtil.obj) : error LNK2001: unresolved external symbol __allmul
Ext4.lib(File.obj) : error LNK2001: unresolved external symbol __allmul
Ext4.lib(Inode.obj) : error LNK2001: unresolved external symbol __allmul
Ext4.lib(BlockGroup.obj) : error LNK2001: unresolved external symbol __allmul
Ext4.lib(Superblock.obj) : error LNK2001: unresolved external symbol __allmul
Ext4.lib(BlockGroup.obj) : error LNK2001: unresolved external symbol __allshl
Ext4.lib(Superblock.obj) : error LNK2001: unresolved external symbol __allshl
Ext4.lib(File.obj) : error LNK2001: unresolved external symbol __allshl
Ext4.lib(Extents.obj) : error LNK2001: unresolved external symbol __allshl
Ext4.lib(Directory.obj) : error LNK2001: unresolved external symbol __allshl
Ext4.lib(Inode.obj) : error LNK2001: unresolved external symbol __allshl

These are usually caused by doing 64-bit math operations when building for IA32.
There are BaseLib functions that do 64-bit path for multiply and left shift that
need to be used when operands are of type INT64 or UINT64.

From the disasm, the following functions are doing 64bit multiple (__allmull)
_Ext4GetFilesystemInfo:
_Ext4FilePhysicalSpace:
_Ext4Read:
_Ext4BlockToByteOffset:
_Ext4ReadInode:
_Ext4OpenSuperblock:
_Ext4ReadBlocks:


From the disasm, the following functions are doing 64-bit shift or power of 2 multiply (__allshl)

_Ext4GetFileInfo:
_Ext4MakeBlockNumberFromHalfs:
_Ext4SetPosition:
_Ext4ExtentIdxLeafBlock:
_Ext4InodeSize:
_Ext4RetrieveDirent:
_Ext4FileATime:
_Ext4FileCrTime:
_Ext4FileMTime:
_Ext4FilePhysicalSpace:
_Ext4InodeSize:
_Ext4Read:
_Ext4MakeBlockNumberFromHalfs:
_Ext4MakeBlockNumberFromHalfs:

For example, Ext4GetFileInfo, uses EXT4_INODE_SIZE() macro, which does a 32-bit shift:

    ext4disk.h:#define EXT4_INODE_SIZE (ino)  (((UINT64)ino->i_size_hi << 32) | ino->i_size_lo)

This can be changes to use the BaseLib function:

    UINT64
    EFIAPI
    LShiftU64 (
      IN      UINT64                    Operand,
      IN      UINTN                     Count
      );

    ext4disk.h:#define EXT4_INODE_SIZE (ino)  ((LShiftU64 (no->i_size_hi, 32) | ino->i_size_lo)

Best regards,

Mike

> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Friday, July 30, 2021 9:17 AM
> To: devel@edk2.groups.io
> Cc: Pedro Falcato <pedro.falcato@gmail.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@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@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@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 (#78719): https://edk2.groups.io/g/devel/message/78719
Mute This Topic: https://groups.io/mt/84553674/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Michael D Kinney 2 years, 7 months ago
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@gmail.com>
> Sent: Friday, July 30, 2021 9:17 AM
> To: devel@edk2.groups.io
> Cc: Pedro Falcato <pedro.falcato@gmail.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@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@nuviainc.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Pedro Falcato 2 years, 7 months ago
Hi Mike,

Thanks for the helpful pointers. I'll consider everything for V2,
which I'll submit as soon as possible (hopefully tomorrow).

RE: Code style. I'll re-run ECC and try to solve the issues. One thing
though: Is it possible to make an exception for the naming of
ext4-specific struct members?
Example: Members' names like "bg_block_bitmap_lo" in
EXT4_BLOCK_GROUP_DESC. I'd like to make a case for it; from my
experience with my own hobby project's ext2 driver, having names
similar to what's used in the documentation/other source code is
incredibly helpful when trying to work on the code; with the original
docs' names, which are admittedly not compliant with the EDK2 coding
style, it really makes everything much clearer when using other code
or documentation as reference. Of course, if it's not possible I'll
rename them all.

Thanks,

Pedro


On Thu, 5 Aug 2021 at 19:33, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> 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@gmail.com>
> > Sent: Friday, July 30, 2021 9:17 AM
> > To: devel@edk2.groups.io
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@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@nuviainc.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Bret Barkelew <Bret.Barkelew@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
>


-- 
Pedro Falcato


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78757): https://edk2.groups.io/g/devel/message/78757
Mute This Topic: https://groups.io/mt/84553674/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
Posted by Michael D Kinney 2 years, 7 months ago

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
> Sent: Thursday, August 5, 2021 3:50 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> Subject: Re: [edk2-devel] [Patch 0/3] Ext4Pkg: Add Ext4Pkg
> 
> Hi Mike,
> 
> Thanks for the helpful pointers. I'll consider everything for V2,
> which I'll submit as soon as possible (hopefully tomorrow).
> 
> RE: Code style. I'll re-run ECC and try to solve the issues. One thing
> though: Is it possible to make an exception for the naming of
> ext4-specific struct members?
> Example: Members' names like "bg_block_bitmap_lo" in

This is ok since this is a structure that is based on the EXT4
documentation,

> EXT4_BLOCK_GROUP_DESC. I'd like to make a case for it; from my
> experience with my own hobby project's ext2 driver, having names
> similar to what's used in the documentation/other source code is
> incredibly helpful when trying to work on the code; with the original
> docs' names, which are admittedly not compliant with the EDK2 coding
> style, it really makes everything much clearer when using other code
> or documentation as reference. Of course, if it's not possible I'll
> rename them all.
> 
> Thanks,
> 
> Pedro
> 
> 
> On Thu, 5 Aug 2021 at 19:33, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >
> > 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@gmail.com>
> > > Sent: Friday, July 30, 2021 9:17 AM
> > > To: devel@edk2.groups.io
> > > Cc: Pedro Falcato <pedro.falcato@gmail.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@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@nuviainc.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Bret Barkelew <Bret.Barkelew@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
> >
> 
> 
> --
> Pedro Falcato
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78816): https://edk2.groups.io/g/devel/message/78816
Mute This Topic: https://groups.io/mt/84553674/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-