Re: [Patch 0/3] Ext4Pkg: Add Ext4Pkg


Michael D Kinney
 

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

Join devel@edk2.groups.io to automatically receive all group messages.