Date   

Re: [edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

Ard Biesheuvel <ardb@...>
 

On Mon, 26 Jul 2021 at 10:59, Yao, Jiewen <jiewen.yao@intel.com> wrote:

Hi

I would like to raise the topic on a confidential computing support in OVMF.



The main target is AMD SEV feature and Intel TDX feature in OVMF package.



The goal is to create a guidance for our future confidential computing work and to better support review and maintenance.
Hello Jiewen,

Thanks for writing this up. As you know, ARM is a bit behind in the
CCA space, and so I will not be able to take part in these discussions
in great detail.

I will leave it to the contributors and other stakeholders to comment
on your proposal below. To me, it looks reasonable.

--
Ard.






[Background]



AMD is adding AMD Secure Encrypted Virtualization (SEV), SEV-Encrypted State (SEV-ES), SEV-Secure Nested Paging (SEV-SNP) features to OVMF package. (https://developer.amd.com/sev/)

Intel is adding Intel Trust Domain Extensions (TDX) features to OVMF package. (https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html)



Both of them support confidential computing use case.



ARM is creating Realm Management Extension (RME). It might be considered in the future. (https://www.arm.com/why-arm/architecture/security-features/arm-confidential-compute-architecture)



So we need a good Confidential Computing infrastructure for EDKII.





[Problem Statement]



1) Current OVMF package integrated some AMD SEV features. But not all features.

Some basic SEV features are in OvmfPkg and enabled as default build. Some advanced SEV features are in OvmfPkg/AmdSev and only enable in AmdSev build.

However, the criteria is NOT clear.



It also brings problem when we want to add more TDX stuff. Where we should go?

For example, I feel PlatformBootManagerLibGrub should be in OvmfPkg/AmdSev. Why it is not there?

https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/PlatformBootManagerLibGrub



We need a clear and consistent rule on where the confidential computing module should go, instead of making ad-hoc decision for each features.



2) Ideally, when we integrate SEV feature or TDX feature, there is some level of isolation, such as

A) standalone driver

B) standalone library

C) standalone file, if it has to be in one module

D) standalone function, if it has to be in one file

The preference is from A to D. A is most preferred. D is least preferred.

As such, when people find something wrong, they can just focus on some SEV or TDX specific files.



We do see good examples, such as SecretDxe (BTW: The name should be SevSecretDxe), AmdSevDxe.

However, some code (ResetVector and Sec) mixed SEV support with normal OVMF code together. That makes it extremely hard to review TDX extensions or maintain a non-SEV code.

https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm

https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c



For latter (such as ResetVector and Sec), I suggest we make a clear isolation. That can help the reviewer to understand better on SEV flow, TDX flow and normal OVMF flow.



3) We may have more problem. For example, how to align the OVMF design between SEV and TDX?

I think, the most SEV OVMF design is good. The TDX OVMF should just follow. For example,

https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)

https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib

https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib





[Proposal]



The current SEV OVMF design is understandable, because when the SEV code was added years ago, it was the first example. We did not know what would be the best way to handle that, and we did not know what TDX would look like.

Today, we have more concrete answer, and let do some refinement step by step.



Confidential computing (CC) == SEV or TDX (it may include RME in the future)



1) CC Feature support (DSC/FDF)



* Try to limit the impact to existing normal OVMF binary.



1.1 - The OVMF packet common DSC/FDF supports OVMF boot in all CC modes or normal mode.



The one OVMF image can boot in normal OVMF mode, SEV mode, or TDX mode.



1.2 - The OVMF packet common DSC/FDF includes *mature* CC feature.



The minimal scope is the image shall boot to OS successfully.

The maximal scope is the feature shall be adopted by OS and will not change for a period of time.



Any immature, under discussion or under review feature shall NOT be put here, such as attestation.



1.3 - The OVMF package CC specific DSC/FDF includes *all* CC feature.



The CC specific DSC/FDF shall be in OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).



The full feature scope may include any feature excluded in 1.2.



Once we believe it is mature and it is well cross-evaluated with other CC infrastructure, this feature may be added to 1.2 later. (step by step approach)



2) CC feature module location



* Try to balance the situation: put too many modules under one dir v.s. create too many layers



2.1 - If it is CC hardware architecture compliant (irrelevant to OVMF), it may be put to non-OvmfPkg.



For example, https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseIoLibIntrinsic



2.2 - If it is a *basic* OVMF CC feature, it shall be put to OvmfPkg directly.



Basic means the OVMF cannot boot without it.



2.3 - If it is an *advanced* OVMF CC feature, it shall be put to OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).



Advanced means the OVMF may still boot without it, just lose some functionality.



3) CC feature convergence.



* Try to help design review and maintenance.



3.1 - A CC feature should be standalone module (driver or library), if it is possible.



Good example:

https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)

https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib

https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib



3.2 - If we have to merge CC feature to a module, then the CC related code shall be isolated to a file.



The file name could be Xxx<Cc>.{c,asm}



A common pattern can be used:



3.2.A - C function.



3.2.A.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:

====================

PreFeatureFunctionHookSev ();

PreFeatureFunctionHookTdx ();

FeatureFunction ();

PostFeatureFunctionHookSev ();

PostFeatureFunctionHookTdx ();

====================

3.2.A.2 - If CC function is a replacement for non-CC function. The main function can check current mode and decide to call which function. For example:

====================

if (IsSev()) {

FeatureFunctionSev();

} else if (IsTdx()) {

FeatureFunctionTdx();

} else {

FeatureFunction();

}

====================



3.2.B - ASM function.



3.2.B.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:

====================

OneTimeCall PreFeatureFunctionHookSev

OneTimeCall PreFeatureFunctionHookTdx

FeatureFunction:

XXXXXX

FeatureFunctionEnd:

OneTimeCall PostMainFunctionHookSev

OneTimeCall PostMainFunctionHookTdx

====================

3.2.B.2 - If CC function is a replacement for non-CC function. The main function can call CC replacement function, then check the return status to decide next step. For example:

====================

OneTimeCallRet FeatureFunctionSev

Jz FeatureFunctionEnd

OneTimeCallRet FeatureFunctionTdx

Jz FeatureFunctionEnd

FeatureFunction:

XXXXXX

FeatureFunctionEnd:

====================





Thank you

Yao Jiewen










[edk2-devel] RFC: Common Design Proposal on Confidential Computing Support in OVMF

Yao, Jiewen
 

Hi
I would like to raise the topic on a confidential computing support in OVMF.

The main target is AMD SEV feature and Intel TDX feature in OVMF package.

The goal is to create a guidance for our future confidential computing work and to better support review and maintenance.


[Background]

AMD is adding AMD Secure Encrypted Virtualization (SEV), SEV-Encrypted State (SEV-ES), SEV-Secure Nested Paging (SEV-SNP) features to OVMF package. (https://developer.amd.com/sev/)
Intel is adding Intel Trust Domain Extensions (TDX) features to OVMF package. (https://software.intel.com/content/www/us/en/develop/articles/intel-trust-domain-extensions.html)

Both of them support confidential computing use case.

ARM is creating Realm Management Extension (RME). It might be considered in the future. (https://www.arm.com/why-arm/architecture/security-features/arm-confidential-compute-architecture)

So we need a good Confidential Computing infrastructure for EDKII.


[Problem Statement]

1) Current OVMF package integrated some AMD SEV features. But not all features.
Some basic SEV features are in OvmfPkg and enabled as default build. Some advanced SEV features are in OvmfPkg/AmdSev and only enable in AmdSev build.
However, the criteria is NOT clear.

It also brings problem when we want to add more TDX stuff. Where we should go?
For example, I feel PlatformBootManagerLibGrub should be in OvmfPkg/AmdSev. Why it is not there?
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/PlatformBootManagerLibGrub

We need a clear and consistent rule on where the confidential computing module should go, instead of making ad-hoc decision for each features.

2) Ideally, when we integrate SEV feature or TDX feature, there is some level of isolation, such as
A) standalone driver
B) standalone library
C) standalone file, if it has to be in one module
D) standalone function, if it has to be in one file
The preference is from A to D. A is most preferred. D is least preferred.
As such, when people find something wrong, they can just focus on some SEV or TDX specific files.

We do see good examples, such as SecretDxe (BTW: The name should be SevSecretDxe), AmdSevDxe.
However, some code (ResetVector and Sec) mixed SEV support with normal OVMF code together. That makes it extremely hard to review TDX extensions or maintain a non-SEV code.
https://github.com/tianocore/edk2/blob/master/OvmfPkg/ResetVector/Ia32/PageTables64.asm
https://github.com/tianocore/edk2/blob/master/OvmfPkg/Sec/SecMain.c

For latter (such as ResetVector and Sec), I suggest we make a clear isolation. That can help the reviewer to understand better on SEV flow, TDX flow and normal OVMF flow.

3) We may have more problem. For example, how to align the OVMF design between SEV and TDX?
I think, the most SEV OVMF design is good. The TDX OVMF should just follow. For example,
https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib


[Proposal]

The current SEV OVMF design is understandable, because when the SEV code was added years ago, it was the first example. We did not know what would be the best way to handle that, and we did not know what TDX would look like.
Today, we have more concrete answer, and let do some refinement step by step.

Confidential computing (CC) == SEV or TDX (it may include RME in the future)

1) CC Feature support (DSC/FDF)

* Try to limit the impact to existing normal OVMF binary.

1.1 - The OVMF packet common DSC/FDF supports OVMF boot in all CC modes or normal mode.

The one OVMF image can boot in normal OVMF mode, SEV mode, or TDX mode.

1.2 - The OVMF packet common DSC/FDF includes *mature* CC feature.

The minimal scope is the image shall boot to OS successfully.
The maximal scope is the feature shall be adopted by OS and will not change for a period of time.

Any immature, under discussion or under review feature shall NOT be put here, such as attestation.

1.3 - The OVMF package CC specific DSC/FDF includes *all* CC feature.

The CC specific DSC/FDF shall be in OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).

The full feature scope may include any feature excluded in 1.2.

Once we believe it is mature and it is well cross-evaluated with other CC infrastructure, this feature may be added to 1.2 later. (step by step approach)

2) CC feature module location

* Try to balance the situation: put too many modules under one dir v.s. create too many layers

2.1 - If it is CC hardware architecture compliant (irrelevant to OVMF), it may be put to non-OvmfPkg.

For example, https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BaseIoLibIntrinsic

2.2 - If it is a *basic* OVMF CC feature, it shall be put to OvmfPkg directly.

Basic means the OVMF cannot boot without it.

2.3 - If it is an *advanced* OVMF CC feature, it shall be put to OvmfPkg/<Cc> (Cc=AmdSev or IntelTdx).

Advanced means the OVMF may still boot without it, just lose some functionality.

3) CC feature convergence.

* Try to help design review and maintenance.

3.1 - A CC feature should be standalone module (driver or library), if it is possible.

Good example:
https://github.com/tianocore/edk2/tree/master/OvmfPkg/IoMmuDxe (BTW: the name should be IoMmuSevDxe)
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/BaseMemEncryptSevLib
https://github.com/tianocore/edk2/tree/master/OvmfPkg/Library/VmgExitLib

3.2 - If we have to merge CC feature to a module, then the CC related code shall be isolated to a file.

The file name could be Xxx<Cc>.{c,asm}

A common pattern can be used:

3.2.A - C function.

3.2.A.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
====================
PreFeatureFunctionHookSev ();
PreFeatureFunctionHookTdx ();
FeatureFunction ();
PostFeatureFunctionHookSev ();
PostFeatureFunctionHookTdx ();
====================
3.2.A.2 - If CC function is a replacement for non-CC function. The main function can check current mode and decide to call which function. For example:
====================
if (IsSev()) {
FeatureFunctionSev();
} else if (IsTdx()) {
FeatureFunctionTdx();
} else {
FeatureFunction();
}
====================

3.2.B - ASM function.

3.2.B.1 - If CC function is a hook, then the main function calls CC function directly. The CC function need implement a CC check function (such as IsSev, or IsTdx). For example:
====================
OneTimeCall PreFeatureFunctionHookSev
OneTimeCall PreFeatureFunctionHookTdx
FeatureFunction:
XXXXXX
FeatureFunctionEnd:
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
3.2.B.2 - If CC function is a replacement for non-CC function. The main function can call CC replacement function, then check the return status to decide next step. For example:
====================
OneTimeCallRet FeatureFunctionSev
Jz FeatureFunctionEnd
OneTimeCallRet FeatureFunctionTdx
Jz FeatureFunctionEnd
FeatureFunction:
XXXXXX
FeatureFunctionEnd:
====================


Thank you
Yao Jiewen


Re: [edk2-devel] RFC: EXT4 filesystem driver

Andrew Fish <afish@...>
 

On Jul 22, 2021, at 7:07 PM, Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> wrote:

Hi Pedro,

-----Original Message-----
From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Pedro
Falcato
Sent: Thursday, July 22, 2021 9:54 AM
To: Andrew Fish <afish@apple.com <mailto:afish@apple.com>>
Cc: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io>>; mhaeuser@posteo.de <mailto:mhaeuser@posteo.de>;
rfc@edk2.groups.io <mailto:rfc@edk2.groups.io>
Subject: Re: [edk2-devel] RFC: EXT4 filesystem driver

Hi Andrew, Marvin,

RE: The package name: It doesn't sound like a bad idea to have something
like a FileSystemPkg and have a bunch of different filesystems inside of it,
but I'll defer to you and my mentors' judgement; we could also drop that
issue for now and take care of it afterwards, since it may need further
changes that are not a part of GSoC and would just delay the process.

With respect to the write capabilities of the driver, I'm not entirely sure
whether or not it's useful. I've been thinking about it today, and it seems like
there's not much that could go wrong? The write path isn't excessively
complex. Except of course in the event of an untimely power cut, but those
/should/ be easily detected by the checksums. My initial idea was to have it
up to speed with FatPkg and other filesystems by implementing all of
EFI_FILE_PROTOCOL, including the write portions. If Apple's HFS+ and APFS
drivers don't have those, it may be a decent idea to reduce the scope of the
ext4 driver as well. I don't see a big need for write support; on the other
hand, I've only worked on UEFI bootloaders before, which may be an outlier
in that regard. Further feedback is appreciated.
The most commonly used reason to for writing to the filesystem in a production environment is capsule updates. Most capsule update implementations will stage the capsule on the EFI System Partition and then reset the system to unlock flash. The second most useful is the UEFI Shell and all the many applications that run within it will write to the filesystem for a large variety of reasons. I think it would be a useful feature to have as one could conceivably start using EFI System Partitions formatted as ext4.
The EFI System Partition is defined to be FAT32 by the UEFI Spec for interoperability. It defines the file system drivers required for the firmware and OS. So changing that is not really an option.

You can still install the UEFI Shell to a read only file system, you just need to do it from the OS :). We actually do this on Macs quite often. You just run the macOS bless command and reboot to the UEFI Shell.

Thanks,

Andrew Fish


As for the tests, UEFI SCTs already seem to have some tests on
EFI_FILE_PROTOCOL's. Further testing may require some sort of fuzzing,
which is what I want to, although in a simplified way. With fuzzing we could
hammer the filesystem code with all sorts of different calls in different
orders, we could also mutate the disk structures to test if the driver is secure
and can handle corruption in a nice, safe way. A future (GSoC or not) project
could also attempt to use compiler-generated coverage instrumentation (see
LLVM's LibFuzzer and SanitizerCoverage for an example).

I'm not sure about all OSes, but at least Linux ext2/3/4 drivers are very robust
and can handle and work around any corrupted FS I
(accidentally) throw at them. However, running fsck is the best way to detect
corruption; note that licensing may be an issue as, for example, ext4's fsck is
GPL2 licensed.

Best Regards,
Pedro

On Thu, 22 Jul 2021 at 16:58, Andrew Fish <afish@apple.com> wrote:



On Jul 22, 2021, at 3:24 AM, Marvin Häuser <mhaeuser@posteo.de> wrote:

On 22.07.21 01:12, Pedro Falcato wrote:

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4
filesystem, due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the
start) the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which
features it supports or not, whether it's been cleanly unmounted, how
many blocks we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group
covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after
the superblock. Each block group descriptor contains the location of
the inode table, and the inode and block bitmaps (note these bitmaps
are only a block long, which gets us the 8 * Block Size formula covered
previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out
referred to as a file, since there is no distinction under the ext4
filesystem) is stored as a /nameless/ inode, that is stored in some
block group's inode table. Each inode has s_inode_size size (or
GOOD_OLD_INODE_SIZE if it's an old filesystem), and holds various
metadata about the file. Since the largest inode structure right now
is ~160 bytes, the rest of the inode contains inline extended
attributes. Inodes' data is stored using either data blocks (under ext2/3) or
extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical
blocks that are represented by N contiguous physical blocks be
represented by a single extent structure, which minimizes filesystem
metadata bloat and speeds up block mapping (particularly due to the
fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0
and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by
logical block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means
ext4 inodes do not themselves have names, since they can be linked
(present) multiple times with different names. Directories can store
entries in two different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem
against system crashes. This is not yet implemented in Ext4Dxe but is
described in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public
documentation available at
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg (relevant discussion:
https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.


While I personally don't like it's outside of the EDK II core, I kind of get it.
However I would strongly suggest to choose a more general package name,
like "LinuxFsPkg", or "NixFsPkg", or maybe even just "FileSystemPkg" (and
move FAT over some day?). Imagine someone wants to import BTRFS next
year, should it really be "BtrfsPkg"? I understand it follows the "FatPkg"
convention, but I feel like people forget FatPkg was special as to its awkward
license before Microsoft allowed a change a few years ago. Maintainers.txt
already has the concept of different Reviewers per subfolder, maybe it could
be extended a little to have a common EDK II contributor to officially maintain
the package, but have you be a Maintainer or something like a Reviewer+ to
your driver? Or you could maintain the entire package of course.


Marvin,

Good point that the FatPkg was more about license boundary than
anything else, so I’m not opposed to a more generic package name.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.


I may be alone, but I strongly object. One of our projects (OpenCore) has a
disgusting way of writing files because the FAT32 driver in Aptio IV firmwares
may corrupt the filesystem when resizing files. To be honest, it may corrupt
with other usages too and we never noticed, because basically we wrote the
code to require the least amount of (complex) FS operations.

The issue with EDK II is, there is a lot of own code and a lot of users, but
little testing. By that I do not mean that developers do not test their code,
but that nobody sits down and performs all sorts of FS manipulations in all
sorts of orders and closely observes the result for regression-testing. Users
will not really test it either, as UEFI to them should just somehow boot to
Windows. If at least the code was shared with a codebase that is known-
trusted (e.g. the Linux kernel itself), that'd be much easier to trust, but
realistically this is not going to happen.
My point is, if a company like AMI cannot guarantee writing does not
damage the FS for a very simple FS, how do you plan to guarantee yours
doesn't for a much more complex FS? I'd rather have only one simple FS type
that supports writing for most use-cases (e.g. logging).

At the very least I would beg you to have a PCD to turn write support
off - if it will be off by default, that'd be great of course. :) Was there any
discussion yet as to why write support is needed in the first place you could
point me to?


I think having a default PCD option of read only is a good idea.

EFI on Mac carries HFS+ and APFS EFI file system drivers and both of those
are read only for safety, security, and to avoid the need to validate them. So I
think some products may want to have the option to ship read only versions
of the file system.

Seems like having EFI base file system tests would be useful. I’d imaging
with OVMF it would be possible to implement a very robust test
infrastructure. Seems like the hard bits would be generating the test cases
and figuring out how to validate the tests did the correct thing. I’m guess the
OS based file system drivers are robust and try to work around bugs
gracefully? Maybe there is a way to turn on OS logging, or even run an OS
based fsck on the volume after the tests complete. Regardless this seems
like an interesting project, maybe we can add it to next years GSoC?

Thanks,

Andrew Fish

Thanks for your work!

Best regards,
Marvin

The driver's handling of unclean unmounting through forced shutdown is
unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato










--
Pedro Falcato



Re: [edk2-devel] RFC: EXT4 filesystem driver

Nate DeSimone
 

Hi Pedro,

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro
Falcato
Sent: Thursday, July 22, 2021 9:54 AM
To: Andrew Fish <afish@apple.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; mhaeuser@posteo.de;
rfc@edk2.groups.io
Subject: Re: [edk2-devel] RFC: EXT4 filesystem driver

Hi Andrew, Marvin,

RE: The package name: It doesn't sound like a bad idea to have something
like a FileSystemPkg and have a bunch of different filesystems inside of it,
but I'll defer to you and my mentors' judgement; we could also drop that
issue for now and take care of it afterwards, since it may need further
changes that are not a part of GSoC and would just delay the process.

With respect to the write capabilities of the driver, I'm not entirely sure
whether or not it's useful. I've been thinking about it today, and it seems like
there's not much that could go wrong? The write path isn't excessively
complex. Except of course in the event of an untimely power cut, but those
/should/ be easily detected by the checksums. My initial idea was to have it
up to speed with FatPkg and other filesystems by implementing all of
EFI_FILE_PROTOCOL, including the write portions. If Apple's HFS+ and APFS
drivers don't have those, it may be a decent idea to reduce the scope of the
ext4 driver as well. I don't see a big need for write support; on the other
hand, I've only worked on UEFI bootloaders before, which may be an outlier
in that regard. Further feedback is appreciated.
The most commonly used reason to for writing to the filesystem in a production environment is capsule updates. Most capsule update implementations will stage the capsule on the EFI System Partition and then reset the system to unlock flash. The second most useful is the UEFI Shell and all the many applications that run within it will write to the filesystem for a large variety of reasons. I think it would be a useful feature to have as one could conceivably start using EFI System Partitions formatted as ext4.


As for the tests, UEFI SCTs already seem to have some tests on
EFI_FILE_PROTOCOL's. Further testing may require some sort of fuzzing,
which is what I want to, although in a simplified way. With fuzzing we could
hammer the filesystem code with all sorts of different calls in different
orders, we could also mutate the disk structures to test if the driver is secure
and can handle corruption in a nice, safe way. A future (GSoC or not) project
could also attempt to use compiler-generated coverage instrumentation (see
LLVM's LibFuzzer and SanitizerCoverage for an example).

I'm not sure about all OSes, but at least Linux ext2/3/4 drivers are very robust
and can handle and work around any corrupted FS I
(accidentally) throw at them. However, running fsck is the best way to detect
corruption; note that licensing may be an issue as, for example, ext4's fsck is
GPL2 licensed.

Best Regards,
Pedro

On Thu, 22 Jul 2021 at 16:58, Andrew Fish <afish@apple.com> wrote:



On Jul 22, 2021, at 3:24 AM, Marvin Häuser <mhaeuser@posteo.de> wrote:

On 22.07.21 01:12, Pedro Falcato wrote:

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4
filesystem, due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the
start) the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which
features it supports or not, whether it's been cleanly unmounted, how
many blocks we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group
covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after
the superblock. Each block group descriptor contains the location of
the inode table, and the inode and block bitmaps (note these bitmaps
are only a block long, which gets us the 8 * Block Size formula covered
previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out
referred to as a file, since there is no distinction under the ext4
filesystem) is stored as a /nameless/ inode, that is stored in some
block group's inode table. Each inode has s_inode_size size (or
GOOD_OLD_INODE_SIZE if it's an old filesystem), and holds various
metadata about the file. Since the largest inode structure right now
is ~160 bytes, the rest of the inode contains inline extended
attributes. Inodes' data is stored using either data blocks (under ext2/3) or
extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical
blocks that are represented by N contiguous physical blocks be
represented by a single extent structure, which minimizes filesystem
metadata bloat and speeds up block mapping (particularly due to the
fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0
and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by
logical block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means
ext4 inodes do not themselves have names, since they can be linked
(present) multiple times with different names. Directories can store
entries in two different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem
against system crashes. This is not yet implemented in Ext4Dxe but is
described in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public
documentation available at
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg (relevant discussion:
https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.


While I personally don't like it's outside of the EDK II core, I kind of get it.
However I would strongly suggest to choose a more general package name,
like "LinuxFsPkg", or "NixFsPkg", or maybe even just "FileSystemPkg" (and
move FAT over some day?). Imagine someone wants to import BTRFS next
year, should it really be "BtrfsPkg"? I understand it follows the "FatPkg"
convention, but I feel like people forget FatPkg was special as to its awkward
license before Microsoft allowed a change a few years ago. Maintainers.txt
already has the concept of different Reviewers per subfolder, maybe it could
be extended a little to have a common EDK II contributor to officially maintain
the package, but have you be a Maintainer or something like a Reviewer+ to
your driver? Or you could maintain the entire package of course.


Marvin,

Good point that the FatPkg was more about license boundary than
anything else, so I’m not opposed to a more generic package name.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.


I may be alone, but I strongly object. One of our projects (OpenCore) has a
disgusting way of writing files because the FAT32 driver in Aptio IV firmwares
may corrupt the filesystem when resizing files. To be honest, it may corrupt
with other usages too and we never noticed, because basically we wrote the
code to require the least amount of (complex) FS operations.

The issue with EDK II is, there is a lot of own code and a lot of users, but
little testing. By that I do not mean that developers do not test their code,
but that nobody sits down and performs all sorts of FS manipulations in all
sorts of orders and closely observes the result for regression-testing. Users
will not really test it either, as UEFI to them should just somehow boot to
Windows. If at least the code was shared with a codebase that is known-
trusted (e.g. the Linux kernel itself), that'd be much easier to trust, but
realistically this is not going to happen.
My point is, if a company like AMI cannot guarantee writing does not
damage the FS for a very simple FS, how do you plan to guarantee yours
doesn't for a much more complex FS? I'd rather have only one simple FS type
that supports writing for most use-cases (e.g. logging).

At the very least I would beg you to have a PCD to turn write support
off - if it will be off by default, that'd be great of course. :) Was there any
discussion yet as to why write support is needed in the first place you could
point me to?


I think having a default PCD option of read only is a good idea.

EFI on Mac carries HFS+ and APFS EFI file system drivers and both of those
are read only for safety, security, and to avoid the need to validate them. So I
think some products may want to have the option to ship read only versions
of the file system.

Seems like having EFI base file system tests would be useful. I’d imaging
with OVMF it would be possible to implement a very robust test
infrastructure. Seems like the hard bits would be generating the test cases
and figuring out how to validate the tests did the correct thing. I’m guess the
OS based file system drivers are robust and try to work around bugs
gracefully? Maybe there is a way to turn on OS logging, or even run an OS
based fsck on the volume after the tests complete. Regardless this seems
like an interesting project, maybe we can add it to next years GSoC?

Thanks,

Andrew Fish

Thanks for your work!

Best regards,
Marvin

The driver's handling of unclean unmounting through forced shutdown is
unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato










--
Pedro Falcato




Re: RFC: EXT4 filesystem driver

Michael D Kinney
 

-----Original Message-----
From: rfc@edk2.groups.io <rfc@edk2.groups.io> On Behalf Of gaoliming
Sent: Wednesday, July 21, 2021 6:21 PM
To: rfc@edk2.groups.io; pedro.falcato@gmail.com; devel@edk2.groups.io
Subject: 回复: [edk2-rfc] RFC: EXT4 filesystem driver



-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2021年7月22日 7:12
收件人: devel@edk2.groups.io
抄送: rfc@edk2.groups.io
主题: [edk2-rfc] RFC: EXT4 filesystem driver

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4 filesystem,
due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the start)
the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which features
it supports or not, whether it's been cleanly unmounted, how many blocks
we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after the
superblock. Each block group descriptor contains the location of the
inode table, and the inode and block bitmaps (note these bitmaps are only
a block long, which gets us the 8 * Block Size formula covered previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out referred
to as a file, since there is no distinction under the ext4 filesystem) is
stored as a /nameless/ inode, that is stored in some block group's inode
table. Each inode has s_inode_size size (or GOOD_OLD_INODE_SIZE if it's
an old filesystem), and holds various metadata about the file. Since the
largest inode structure right now is ~160 bytes, the rest of the inode
contains inline extended attributes. Inodes' data is stored using either
data blocks (under ext2/3) or extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical blocks
that are represented by N contiguous physical blocks be represented by a
single extent structure, which minimizes filesystem metadata bloat and
speeds up block mapping (particularly due to the fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0
and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by logical
block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means ext4
inodes do not themselves have names, since they can be linked (present)
multiple times with different names. Directories can store entries in two
different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem against
system crashes. This is not yet implemented in Ext4Dxe but is described
in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public documentation
available at
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg
(relevant discussion: https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with
those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.

The driver's handling of unclean unmounting through forced shutdown is
unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?
Current MdePkg BaseLib has CalculateCrc32(). So, CRC32C and CRC16 can be added into BaseLib.
I agree BaseLib is as good location for these 2 APIs.


If more modules need to consume Ucs2 <-> Utf8 conversion library, BaseUcs2Utf8Lib is generic enough
to be placed in MdePkg.
This was discussed with the RedfishPkg maintainers and was deferred until there was another use
case. Looks like EXT4 is a 2nd use case, so it is a good time to move to this lib class and
lib instance from RedfishPkg to MdePkg.


Thanks
Liming

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato









Re: [edk2-devel] RFC: EXT4 filesystem driver

Pedro Falcato
 

Hi Andrew, Marvin,

RE: The package name: It doesn't sound like a bad idea to have
something like a FileSystemPkg and have a bunch of different
filesystems inside of it, but I'll defer to you
and my mentors' judgement; we could also drop that issue for now and
take care of it afterwards, since it may need further changes that are
not a part of GSoC and would just delay the process.

With respect to the write capabilities of the driver, I'm not entirely
sure whether or not it's useful. I've been thinking about it today,
and it seems like there's not much that could go wrong? The write path
isn't excessively complex. Except of course in the event of an
untimely power cut, but those /should/ be easily detected by the
checksums. My initial idea was to have it up to speed with FatPkg and
other filesystems by implementing all of EFI_FILE_PROTOCOL, including
the write portions. If Apple's HFS+ and APFS drivers don't have those,
it may be a decent idea to reduce the scope of the ext4 driver as
well. I don't see a big need for write support; on the other hand,
I've only worked on UEFI bootloaders before, which may be an outlier
in that regard. Further feedback is appreciated.

As for the tests, UEFI SCTs already seem to have some tests on
EFI_FILE_PROTOCOL's. Further testing may require some sort of fuzzing,
which is what I want to, although in a
simplified way. With fuzzing we could hammer the filesystem code with
all sorts of different calls in different orders, we could also mutate
the disk structures to test if the driver is secure and
can handle corruption in a nice, safe way. A future (GSoC or not)
project could also attempt to use compiler-generated coverage
instrumentation (see LLVM's LibFuzzer and SanitizerCoverage for an
example).

I'm not sure about all OSes, but at least Linux ext2/3/4 drivers are
very robust and can handle and work around any corrupted FS I
(accidentally) throw at them. However, running fsck is the best way to
detect corruption; note that licensing may be an issue as, for
example, ext4's fsck is GPL2 licensed.

Best Regards,
Pedro

On Thu, 22 Jul 2021 at 16:58, Andrew Fish <afish@apple.com> wrote:



On Jul 22, 2021, at 3:24 AM, Marvin Häuser <mhaeuser@posteo.de> wrote:

On 22.07.21 01:12, Pedro Falcato wrote:

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4 filesystem,
due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the start)
the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which features
it supports or not, whether it's been cleanly unmounted, how many blocks
we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after the
superblock. Each block group descriptor contains the location of the
inode table, and the inode and block bitmaps (note these bitmaps are only
a block long, which gets us the 8 * Block Size formula covered previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out referred
to as a file, since there is no distinction under the ext4 filesystem) is
stored as a /nameless/ inode, that is stored in some block group's inode
table. Each inode has s_inode_size size (or GOOD_OLD_INODE_SIZE if it's
an old filesystem), and holds various metadata about the file. Since the
largest inode structure right now is ~160 bytes, the rest of the inode
contains inline extended attributes. Inodes' data is stored using either
data blocks (under ext2/3) or extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical blocks
that are represented by N contiguous physical blocks be represented by a
single extent structure, which minimizes filesystem metadata bloat and
speeds up block mapping (particularly due to the fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0 and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by logical
block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means ext4
inodes do not themselves have names, since they can be linked (present)
multiple times with different names. Directories can store entries in two
different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem against
system crashes. This is not yet implemented in Ext4Dxe but is described
in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public documentation
available at https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg
(relevant discussion: https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.


While I personally don't like it's outside of the EDK II core, I kind of get it. However I would strongly suggest to choose a more general package name, like "LinuxFsPkg", or "NixFsPkg", or maybe even just "FileSystemPkg" (and move FAT over some day?). Imagine someone wants to import BTRFS next year, should it really be "BtrfsPkg"? I understand it follows the "FatPkg" convention, but I feel like people forget FatPkg was special as to its awkward license before Microsoft allowed a change a few years ago. Maintainers.txt already has the concept of different Reviewers per subfolder, maybe it could be extended a little to have a common EDK II contributor to officially maintain the package, but have you be a Maintainer or something like a Reviewer+ to your driver? Or you could maintain the entire package of course.


Marvin,

Good point that the FatPkg was more about license boundary than anything else, so I’m not opposed to a more generic package name.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with
those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.


I may be alone, but I strongly object. One of our projects (OpenCore) has a disgusting way of writing files because the FAT32 driver in Aptio IV firmwares may corrupt the filesystem when resizing files. To be honest, it may corrupt with other usages too and we never noticed, because basically we wrote the code to require the least amount of (complex) FS operations.

The issue with EDK II is, there is a lot of own code and a lot of users, but little testing. By that I do not mean that developers do not test their code, but that nobody sits down and performs all sorts of FS manipulations in all sorts of orders and closely observes the result for regression-testing. Users will not really test it either, as UEFI to them should just somehow boot to Windows. If at least the code was shared with a codebase that is known-trusted (e.g. the Linux kernel itself), that'd be much easier to trust, but realistically this is not going to happen.
My point is, if a company like AMI cannot guarantee writing does not damage the FS for a very simple FS, how do you plan to guarantee yours doesn't for a much more complex FS? I'd rather have only one simple FS type that supports writing for most use-cases (e.g. logging).

At the very least I would beg you to have a PCD to turn write support off - if it will be off by default, that'd be great of course. :)
Was there any discussion yet as to why write support is needed in the first place you could point me to?


I think having a default PCD option of read only is a good idea.

EFI on Mac carries HFS+ and APFS EFI file system drivers and both of those are read only for safety, security, and to avoid the need to validate them. So I think some products may want to have the option to ship read only versions of the file system.

Seems like having EFI base file system tests would be useful. I’d imaging with OVMF it would be possible to implement a very robust test infrastructure. Seems like the hard bits would be generating the test cases and figuring out how to validate the tests did the correct thing. I’m guess the OS based file system drivers are robust and try to work around bugs gracefully? Maybe there is a way to turn on OS logging, or even run an OS based fsck on the volume after the tests complete. Regardless this seems like an interesting project, maybe we can add it to next years GSoC?

Thanks,

Andrew Fish

Thanks for your work!

Best regards,
Marvin

The driver's handling of unclean unmounting through forced shutdown is unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato









--
Pedro Falcato


Re: [edk2-devel] [edk2-rfc] RFC: EXT4 filesystem driver

Andrew Fish <afish@...>
 

On Jul 21, 2021, at 6:20 PM, gaoliming <gaoliming@byosoft.com.cn> wrote:



-----邮件原件-----
发件人: rfc@edk2.groups.io <mailto:rfc@edk2.groups.io> <rfc@edk2.groups.io <mailto:rfc@edk2.groups.io>> 代表 Pedro Falcato
发送时间: 2021年7月22日 7:12
收件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
抄送: rfc@edk2.groups.io <mailto:rfc@edk2.groups.io>
主题: [edk2-rfc] RFC: EXT4 filesystem driver

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4 filesystem,
due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the start)
the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which features
it supports or not, whether it's been cleanly unmounted, how many blocks
we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after the
superblock. Each block group descriptor contains the location of the
inode table, and the inode and block bitmaps (note these bitmaps are only
a block long, which gets us the 8 * Block Size formula covered previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out referred
to as a file, since there is no distinction under the ext4 filesystem) is
stored as a /nameless/ inode, that is stored in some block group's inode
table. Each inode has s_inode_size size (or GOOD_OLD_INODE_SIZE if it's
an old filesystem), and holds various metadata about the file. Since the
largest inode structure right now is ~160 bytes, the rest of the inode
contains inline extended attributes. Inodes' data is stored using either
data blocks (under ext2/3) or extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical blocks
that are represented by N contiguous physical blocks be represented by a
single extent structure, which minimizes filesystem metadata bloat and
speeds up block mapping (particularly due to the fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0
and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by logical
block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means ext4
inodes do not themselves have names, since they can be linked (present)
multiple times with different names. Directories can store entries in two
different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem against
system crashes. This is not yet implemented in Ext4Dxe but is described
in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public documentation
available at
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg
(relevant discussion: https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with
those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.

The driver's handling of unclean unmounting through forced shutdown is
unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?
Current MdePkg BaseLib has CalculateCrc32(). So, CRC32C and CRC16 can be added into BaseLib.

If more modules need to consume Ucs2 <-> Utf8 conversion library, BaseUcs2Utf8Lib is generic enough
to be placed in MdePkg.
I think the Terminal driver may have some similar logic to convert UTF-8 terminals to/from the UEFI UCS-2?

https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Console/TerminalDxe/Vtutf8.c#L186

Thanks,

Andrew Fish

Thanks
Liming

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato








回复: [edk2-rfc] RFC: EXT4 filesystem driver

gaoliming
 

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2021年7月22日 7:12
收件人: devel@edk2.groups.io
抄送: rfc@edk2.groups.io
主题: [edk2-rfc] RFC: EXT4 filesystem driver

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4 filesystem,
due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the start)
the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which features
it supports or not, whether it's been cleanly unmounted, how many blocks
we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after the
superblock. Each block group descriptor contains the location of the
inode table, and the inode and block bitmaps (note these bitmaps are only
a block long, which gets us the 8 * Block Size formula covered previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out referred
to as a file, since there is no distinction under the ext4 filesystem) is
stored as a /nameless/ inode, that is stored in some block group's inode
table. Each inode has s_inode_size size (or GOOD_OLD_INODE_SIZE if it's
an old filesystem), and holds various metadata about the file. Since the
largest inode structure right now is ~160 bytes, the rest of the inode
contains inline extended attributes. Inodes' data is stored using either
data blocks (under ext2/3) or extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical blocks
that are represented by N contiguous physical blocks be represented by a
single extent structure, which minimizes filesystem metadata bloat and
speeds up block mapping (particularly due to the fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0
and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by logical
block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means ext4
inodes do not themselves have names, since they can be linked (present)
multiple times with different names. Directories can store entries in two
different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem against
system crashes. This is not yet implemented in Ext4Dxe but is described
in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public documentation
available at
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg
(relevant discussion: https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with
those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.

The driver's handling of unclean unmounting through forced shutdown is
unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?
Current MdePkg BaseLib has CalculateCrc32(). So, CRC32C and CRC16 can be added into BaseLib.

If more modules need to consume Ucs2 <-> Utf8 conversion library, BaseUcs2Utf8Lib is generic enough
to be placed in MdePkg.

Thanks
Liming

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato




RFC: EXT4 filesystem driver

Pedro Falcato
 

EXT4 (fourth extended filesystem) is a filesystem developed for Linux
that has been in wide use (desktops, servers, smartphones) since 2008.

The Ext4Pkg implements the Simple File System Protocol for a partition
that is formatted with the EXT4 file system. This allows UEFI Drivers,
UEFI Applications, UEFI OS Loaders, and the UEFI Shell to access files
on an EXT4 partition and supports booting a UEFI OS Loader from an
EXT4 partition.
This project is one of the TianoCore Google Summer of Code projects.

Right now, Ext4Pkg only contains a single member, Ext4Dxe, which is a
UEFI driver that consumes Block I/O, Disk I/O and (optionally) Disk
I/O 2 Protocols, and produces the Simple File System protocol. It
allows mounting ext4 filesystems exclusively.

Brief overhead of EXT4:
Layout of an EXT2/3/4 filesystem:
(note: this driver has been developed using
https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html as
documentation).

An ext2/3/4 filesystem (here on out referred to as simply an ext4 filesystem,
due to the similarities) is composed of various concepts:

1) Superblock
The superblock is the structure near (1024 bytes offset from the start)
the start of the partition, and describes the filesystem in general.
Here, we get to know the size of the filesystem's blocks, which features
it supports or not, whether it's been cleanly unmounted, how many blocks
we have, etc.

2) Block groups
EXT4 filesystems are divided into block groups, and each block group covers
s_blocks_per_group(8 * Block Size) blocks. Each block group has an
associated block group descriptor; these are present directly after the
superblock. Each block group descriptor contains the location of the
inode table, and the inode and block bitmaps (note these bitmaps are only
a block long, which gets us the 8 * Block Size formula covered previously).

3) Blocks
The ext4 filesystem is divided into blocks, of size s_log_block_size ^ 1024.
Blocks can be allocated using individual block groups's bitmaps. Note
that block 0 is invalid and its presence on extents/block tables means
it's part of a file hole, and that particular location must be read as
a block full of zeros.

4) Inodes
The ext4 filesystem divides files/directories into inodes (originally
index nodes). Each file/socket/symlink/directory/etc (here on out referred
to as a file, since there is no distinction under the ext4 filesystem) is
stored as a /nameless/ inode, that is stored in some block group's inode
table. Each inode has s_inode_size size (or GOOD_OLD_INODE_SIZE if it's
an old filesystem), and holds various metadata about the file. Since the
largest inode structure right now is ~160 bytes, the rest of the inode
contains inline extended attributes. Inodes' data is stored using either
data blocks (under ext2/3) or extents (under ext4).

5) Extents
Ext4 inodes store data in extents. These let N contiguous logical blocks
that are represented by N contiguous physical blocks be represented by a
single extent structure, which minimizes filesystem metadata bloat and
speeds up block mapping (particularly due to the fact that high-quality
ext4 implementations like linux's try /really/ hard to make the file
contiguous, so it's common to have files with almost 0 fragmentation).
Inodes that use extents store them in a tree, and the top of the tree
is stored on i_data. The tree's leaves always start with an
EXT4_EXTENT_HEADER and contain EXT4_EXTENT_INDEX on eh_depth != 0 and
EXT4_EXTENT on eh_depth = 0; these entries are always sorted by logical
block.

6) Directories
Ext4 directories are files that store name -> inode mappings for the
logical directory; this is where files get their names, which means ext4
inodes do not themselves have names, since they can be linked (present)
multiple times with different names. Directories can store entries in two
different ways:
1) Classical linear directories: They store entries as a mostly-linked
mostly-list of EXT4_DIR_ENTRY.
2) Hash tree directories: These are used for larger directories, with
hundreds of entries, and are designed in a backwards-compatible way.
These are not yet implemented in the Ext4Dxe driver.

7) Journal
Ext3/4 filesystems have a journal to help protect the filesystem against
system crashes. This is not yet implemented in Ext4Dxe but is described
in detail in the Linux kernel's documentation.

The EDK2 implementation of ext4 is based only on the public documentation
available at https://www.kernel.org/doc/html/latest/filesystems/ext4/index.html
and
the FreeBSD ext2fs driver (available at
https://github.com/freebsd/freebsd-src/tree/main/sys/fs/ext2fs,
BSD-2-Clause-FreeBSD licensed). It is licensed as
SPDX-License-Identifier: BSD-2-Clause-Patent.

After a brief discussion with the community, the proposed package
location is edk2-platform/Features/Ext4Pkg
(relevant discussion: https://edk2.groups.io/g/devel/topic/83060185).

I was the main contributor and I would like to maintain the package in
the future, if possible.

Current limitations:
1) The Ext4Dxe driver is, at the moment, read-only.
2) The Ext4Dxe driver at the moment cannot mount older (ext2/3)
filesystems. Ensuring compatibility with
those may not be a bad idea.

I intend to test the package using the UEFI SCTs present in edk2-test,
and implement any other needed unit tests myself using the already
available unit test framework. I also intend to (privately) fuzz the
UEFI driver with bad/unusual disk images, to improve the security and
reliability of the driver.

In the future, ext4 write support should be added so edk2 has a
fully-featured RW ext4 driver. There could also be a focus on
supporting the older ext4-like filesystems, as I mentioned in the
limitations, but that is open for discussion.

The driver's handling of unclean unmounting through forced shutdown is unclear.
Is there a position in edk2 on how to handle such cases? I don't think
FAT32 has a "this filesystem is/was dirty" and even though it seems to
me that stopping a system from booting/opening the partition because
"we may find some tiny irregularities" is not the best course of
action, I can't find a clear answer.

The driver also had to add implementations of CRC32C and CRC16, and
after talking with my mentor we quickly reached the conclusion that
these may be good candidates for inclusion in MdePkg. We also
discussed moving the Ucs2 <-> Utf8 conversion library in RedfishPkg
(BaseUcs2Utf8Lib) into MdePkg as well. Any comments?

Feel free to ask any questions you may find relevant.

Best Regards,

Pedro Falcato


回复: 回复: 回复: [edk2-rfc] release candidate tags

gaoliming
 

There is no other comment for this proposal. I will send it to edk2 devel mail list to collect the feedback.

Thanks
Liming

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月29日 16:23
收件人: rfc@edk2.groups.io; gaoliming@byosoft.com.cn
主题: Re: 回复: 回复: [edk2-rfc] release candidate tags

On 06/29/21 04:12, gaoliming wrote:
Laszlo:
OK. I give the new proposed date for the release planning. SFF will be
shorten to 5 days. HFF will be extended to 14 days.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-13 Hard Feature Freeze
2021-08-27 Release
Thank you. A shorter SFF should not be a problem, as we use it anyway
only for merging previously reviewed feature patch sets.

I hope other community members are OK with this as well.

Thanks!
Laszlo


Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月28日 22:19
收件人: gaoliming <gaoliming@byosoft.com.cn>; rfc@edk2.groups.io
主题: Re: 回复: [edk2-rfc] release candidate tags

On 06/25/21 04:11, gaoliming wrote:
Laszlo:
I understand this release requirement. Now, we have Feature Planning
Freeze (1WW), Soft Feature Freeze (1WW) and Hard Feature Freeze (5
days). I
would propose to remove Feature Planning Freeze, keep Soft Feature
Freeze
(1WW), and extend Hard Feature Freeze (1week and 5 days). For Q3 stable
tag, new planning will be like the below.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-16 Hard Feature Freeze
2021-08-27 Release
I haven't even been aware of the "planning freeze". I think we can
remove it.

Regarding the HFF, I'd really suggest / request 14 calendar days.

Thanks
Laszlo






Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月22日 18:24
收件人: rfc@edk2.groups.io
抄送: Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>
主题: [edk2-rfc] release candidate tags

Hi,

(1) I'm proposing an extension to the soft feature freeze and hard
feature freeze announcements:


https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze#an
nouncing-the-soft-feature-freeze

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze#a
nnouncing-the-hard-feature-freeze

as follows:

When the SFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag
of
the form

edk2-stableYYYYMM-rc0

When the HFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag
of
the form

edk2-stableYYYYMM-rc1

Note that a single commit may bear multiple tags in the end; for
example, if there are no fixes merged between the HFF announcement
and
the actual release, then the final commit would bear both tags

edk2-stableYYYYMM-rc1
edk2-stableYYYYMM


The purpose of the Release Candidate tags is to coordinate pre-release
testing between consumers (downstreams) of edk2. Concentrated
pre-release testing is useful because it helps downstreams (a) identify
issues against a common base and (b) contribute upstream bugfixes still
in time for the actual release.


(2) Relatedly, I'm proposing that the Hard Feature Freeze never be
shorter than 2 calendar weeks.

Background: if I recall correctly, the Hard Feature Freeze for
edk2-stable202105 was 4 days. That's not enough for the
above-described,
downstream, pre-release testing. In my opinion, two calendar weeks are
sensible for the "finishing touches" on the release.

I'm not asking for an extended Soft Feature Freeze. I reckon that most
downstreams will want to start their pre-release integration and testing
at the rc1 tag. Between the rc0 and rc1 tags (that is, during the Soft
Feature Freeze), features reviewed previously may still be merged, and
those have a higher chance to invalidate downstream testing performed
earlier. So the "real" testing will likely commence at rc1, and so the
period we'd extend to 2 calendar weeks should be the Hard Feature
Freeze.

(I'm not expressing the new period length in "business days", as the
definition of those varies around the world, and over time.)

Thanks,
Laszlo


















Re: [edk2-devel] RFC: design review for TDVF in OVMF

Min Xu <min.m.xu@...>
 

Thanks much everyone who attended 2 sessions of TDVF design review meeting
and lots of valuable comments and feedbacks received. These 2 meetings were
recorded and now uploaded to below link:
Session 1:
https://drive.google.com/file/d/100__tNVe5erNzExySq2SJOprvBN7zz8u/view?usp=sharing
Session 2:
https://drive.google.com/file/d/1aDvtLhLxzniOISljXwjZH0YT_m7EBn8b/view?usp=sharing

Thank you!
Min


Re: 回复: 回复: [edk2-rfc] release candidate tags

Laszlo Ersek
 

On 06/29/21 04:12, gaoliming wrote:
Laszlo:
OK. I give the new proposed date for the release planning. SFF will be shorten to 5 days. HFF will be extended to 14 days.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-13 Hard Feature Freeze
2021-08-27 Release
Thank you. A shorter SFF should not be a problem, as we use it anyway
only for merging previously reviewed feature patch sets.

I hope other community members are OK with this as well.

Thanks!
Laszlo


Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月28日 22:19
收件人: gaoliming <gaoliming@byosoft.com.cn>; rfc@edk2.groups.io
主题: Re: 回复: [edk2-rfc] release candidate tags

On 06/25/21 04:11, gaoliming wrote:
Laszlo:
I understand this release requirement. Now, we have Feature Planning
Freeze (1WW), Soft Feature Freeze (1WW) and Hard Feature Freeze (5 days). I
would propose to remove Feature Planning Freeze, keep Soft Feature Freeze
(1WW), and extend Hard Feature Freeze (1week and 5 days). For Q3 stable
tag, new planning will be like the below.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-16 Hard Feature Freeze
2021-08-27 Release
I haven't even been aware of the "planning freeze". I think we can
remove it.

Regarding the HFF, I'd really suggest / request 14 calendar days.

Thanks
Laszlo






Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月22日 18:24
收件人: rfc@edk2.groups.io
抄送: Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>
主题: [edk2-rfc] release candidate tags

Hi,

(1) I'm proposing an extension to the soft feature freeze and hard
feature freeze announcements:


https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze#an
nouncing-the-soft-feature-freeze

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze#a
nnouncing-the-hard-feature-freeze

as follows:

When the SFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc0

When the HFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc1

Note that a single commit may bear multiple tags in the end; for
example, if there are no fixes merged between the HFF announcement and
the actual release, then the final commit would bear both tags

edk2-stableYYYYMM-rc1
edk2-stableYYYYMM


The purpose of the Release Candidate tags is to coordinate pre-release
testing between consumers (downstreams) of edk2. Concentrated
pre-release testing is useful because it helps downstreams (a) identify
issues against a common base and (b) contribute upstream bugfixes still
in time for the actual release.


(2) Relatedly, I'm proposing that the Hard Feature Freeze never be
shorter than 2 calendar weeks.

Background: if I recall correctly, the Hard Feature Freeze for
edk2-stable202105 was 4 days. That's not enough for the above-described,
downstream, pre-release testing. In my opinion, two calendar weeks are
sensible for the "finishing touches" on the release.

I'm not asking for an extended Soft Feature Freeze. I reckon that most
downstreams will want to start their pre-release integration and testing
at the rc1 tag. Between the rc0 and rc1 tags (that is, during the Soft
Feature Freeze), features reviewed previously may still be merged, and
those have a higher chance to invalidate downstream testing performed
earlier. So the "real" testing will likely commence at rc1, and so the
period we'd extend to 2 calendar weeks should be the Hard Feature
Freeze.

(I'm not expressing the new period length in "business days", as the
definition of those varies around the world, and over time.)

Thanks,
Laszlo















回复: 回复: [edk2-rfc] release candidate tags

gaoliming
 

Laszlo:
OK. I give the new proposed date for the release planning. SFF will be shorten to 5 days. HFF will be extended to 14 days.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-13 Hard Feature Freeze
2021-08-27 Release

Thanks
Liming

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月28日 22:19
收件人: gaoliming <gaoliming@byosoft.com.cn>; rfc@edk2.groups.io
主题: Re: 回复: [edk2-rfc] release candidate tags

On 06/25/21 04:11, gaoliming wrote:
Laszlo:
I understand this release requirement. Now, we have Feature Planning
Freeze (1WW), Soft Feature Freeze (1WW) and Hard Feature Freeze (5 days). I
would propose to remove Feature Planning Freeze, keep Soft Feature Freeze
(1WW), and extend Hard Feature Freeze (1week and 5 days). For Q3 stable
tag, new planning will be like the below.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-16 Hard Feature Freeze
2021-08-27 Release
I haven't even been aware of the "planning freeze". I think we can
remove it.

Regarding the HFF, I'd really suggest / request 14 calendar days.

Thanks
Laszlo






Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月22日 18:24
收件人: rfc@edk2.groups.io
抄送: Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>
主题: [edk2-rfc] release candidate tags

Hi,

(1) I'm proposing an extension to the soft feature freeze and hard
feature freeze announcements:


https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze#an
nouncing-the-soft-feature-freeze

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze#a
nnouncing-the-hard-feature-freeze

as follows:

When the SFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc0

When the HFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc1

Note that a single commit may bear multiple tags in the end; for
example, if there are no fixes merged between the HFF announcement and
the actual release, then the final commit would bear both tags

edk2-stableYYYYMM-rc1
edk2-stableYYYYMM


The purpose of the Release Candidate tags is to coordinate pre-release
testing between consumers (downstreams) of edk2. Concentrated
pre-release testing is useful because it helps downstreams (a) identify
issues against a common base and (b) contribute upstream bugfixes still
in time for the actual release.


(2) Relatedly, I'm proposing that the Hard Feature Freeze never be
shorter than 2 calendar weeks.

Background: if I recall correctly, the Hard Feature Freeze for
edk2-stable202105 was 4 days. That's not enough for the above-described,
downstream, pre-release testing. In my opinion, two calendar weeks are
sensible for the "finishing touches" on the release.

I'm not asking for an extended Soft Feature Freeze. I reckon that most
downstreams will want to start their pre-release integration and testing
at the rc1 tag. Between the rc0 and rc1 tags (that is, during the Soft
Feature Freeze), features reviewed previously may still be merged, and
those have a higher chance to invalidate downstream testing performed
earlier. So the "real" testing will likely commence at rc1, and so the
period we'd extend to 2 calendar weeks should be the Hard Feature
Freeze.

(I'm not expressing the new period length in "business days", as the
definition of those varies around the world, and over time.)

Thanks,
Laszlo









Re: 回复: [edk2-rfc] release candidate tags

Laszlo Ersek
 

On 06/25/21 04:11, gaoliming wrote:
Laszlo:
I understand this release requirement. Now, we have Feature Planning Freeze (1WW), Soft Feature Freeze (1WW) and Hard Feature Freeze (5 days). I would propose to remove Feature Planning Freeze, keep Soft Feature Freeze (1WW), and extend Hard Feature Freeze (1week and 5 days). For Q3 stable tag, new planning will be like the below.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-16 Hard Feature Freeze
2021-08-27 Release
I haven't even been aware of the "planning freeze". I think we can
remove it.

Regarding the HFF, I'd really suggest / request 14 calendar days.

Thanks
Laszlo






Thanks
Liming
-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月22日 18:24
收件人: rfc@edk2.groups.io
抄送: Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>
主题: [edk2-rfc] release candidate tags

Hi,

(1) I'm proposing an extension to the soft feature freeze and hard
feature freeze announcements:


https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze#an
nouncing-the-soft-feature-freeze

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze#a
nnouncing-the-hard-feature-freeze

as follows:

When the SFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc0

When the HFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc1

Note that a single commit may bear multiple tags in the end; for
example, if there are no fixes merged between the HFF announcement and
the actual release, then the final commit would bear both tags

edk2-stableYYYYMM-rc1
edk2-stableYYYYMM


The purpose of the Release Candidate tags is to coordinate pre-release
testing between consumers (downstreams) of edk2. Concentrated
pre-release testing is useful because it helps downstreams (a) identify
issues against a common base and (b) contribute upstream bugfixes still
in time for the actual release.


(2) Relatedly, I'm proposing that the Hard Feature Freeze never be
shorter than 2 calendar weeks.

Background: if I recall correctly, the Hard Feature Freeze for
edk2-stable202105 was 4 days. That's not enough for the above-described,
downstream, pre-release testing. In my opinion, two calendar weeks are
sensible for the "finishing touches" on the release.

I'm not asking for an extended Soft Feature Freeze. I reckon that most
downstreams will want to start their pre-release integration and testing
at the rc1 tag. Between the rc0 and rc1 tags (that is, during the Soft
Feature Freeze), features reviewed previously may still be merged, and
those have a higher chance to invalidate downstream testing performed
earlier. So the "real" testing will likely commence at rc1, and so the
period we'd extend to 2 calendar weeks should be the Hard Feature
Freeze.

(I'm not expressing the new period length in "business days", as the
definition of those varies around the world, and over time.)

Thanks,
Laszlo






回复: [edk2-rfc] release candidate tags

gaoliming
 

Laszlo:
I understand this release requirement. Now, we have Feature Planning Freeze (1WW), Soft Feature Freeze (1WW) and Hard Feature Freeze (5 days). I would propose to remove Feature Planning Freeze, keep Soft Feature Freeze (1WW), and extend Hard Feature Freeze (1week and 5 days). For Q3 stable tag, new planning will be like the below.

Date (00:00:00 UTC-8) Description
2021-05-28 Beginning of development
2021-08-09 Soft Feature Freeze
2021-08-16 Hard Feature Freeze
2021-08-27 Release

Thanks
Liming

-----邮件原件-----
发件人: rfc@edk2.groups.io <rfc@edk2.groups.io> 代表 Laszlo Ersek
发送时间: 2021年6月22日 18:24
收件人: rfc@edk2.groups.io
抄送: Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>
主题: [edk2-rfc] release candidate tags

Hi,

(1) I'm proposing an extension to the soft feature freeze and hard
feature freeze announcements:


https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze#an
nouncing-the-soft-feature-freeze

https://github.com/tianocore/tianocore.github.io/wiki/HardFeatureFreeze#a
nnouncing-the-hard-feature-freeze

as follows:

When the SFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc0

When the HFF is announced, the Release Manager should please tag the
then-HEAD commit of the "master" branch with a Release Candidate tag of
the form

edk2-stableYYYYMM-rc1

Note that a single commit may bear multiple tags in the end; for
example, if there are no fixes merged between the HFF announcement and
the actual release, then the final commit would bear both tags

edk2-stableYYYYMM-rc1
edk2-stableYYYYMM


The purpose of the Release Candidate tags is to coordinate pre-release
testing between consumers (downstreams) of edk2. Concentrated
pre-release testing is useful because it helps downstreams (a) identify
issues against a common base and (b) contribute upstream bugfixes still
in time for the actual release.


(2) Relatedly, I'm proposing that the Hard Feature Freeze never be
shorter than 2 calendar weeks.

Background: if I recall correctly, the Hard Feature Freeze for
edk2-stable202105 was 4 days. That's not enough for the above-described,
downstream, pre-release testing. In my opinion, two calendar weeks are
sensible for the "finishing touches" on the release.

I'm not asking for an extended Soft Feature Freeze. I reckon that most
downstreams will want to start their pre-release integration and testing
at the rc1 tag. Between the rc0 and rc1 tags (that is, during the Soft
Feature Freeze), features reviewed previously may still be merged, and
those have a higher chance to invalidate downstream testing performed
earlier. So the "real" testing will likely commence at rc1, and so the
period we'd extend to 2 calendar weeks should be the Hard Feature
Freeze.

(I'm not expressing the new period length in "business days", as the
definition of those varies around the world, and over time.)

Thanks,
Laszlo





Re: [edk2-devel] RFC: design review for TDVF in OVMF

Min Xu <min.m.xu@...>
 

On 06/24/2021 8:36 AM, James Bottomley wrote:
On Thu, 2021-06-24 at 00:24 +0000, Min Xu wrote:
On 06/22/2021 9:39 PM, Laszlo wrote:
I should clarify: the relevant part of my preference is not that
"IntelTdx.dsc"
contain the *complete* TDVF feature set. The relevant part (for me)
is that "OvmfPkgX64.dsc" *not* be over-complicated for the sake of
TDX, even considering only the "basic" TDVF feature set. It's fine
to implement TDX in two stages ("basic" and "complete"); my point is
that even "basic"
should not over-
complicate "OvmfPkgX64.dsc".
Thanks much for the comments and we don't want to make OvmfPkgX64.dsc
over-complicated either.
We have updated the design slides to V0.95 and Slides 6-15 are
discussing the Config-A and Config-B.
https://edk2.groups.io/g/devel/files/Designs/2021/0611/TDVF_Design_Rev
iew%28v0.95%29.pptx
Your comment is always welcome!
The mailing list still won't give me that file, can you update it in the bugzilla:

https://bugzilla.tianocore.org/show_bug.cgi?id=3429

As well, please?
Sure. TDVF Design Review v0.95 is uploaded to
https://bugzilla.tianocore.org/show_bug.cgi?id=3429
Thanks
Min


Re: [edk2-devel] RFC: design review for TDVF in OVMF

James Bottomley <jejb@...>
 

On Thu, 2021-06-24 at 00:24 +0000, Min Xu wrote:
On 06/22/2021 9:39 PM, Laszlo wrote:
I should clarify: the relevant part of my preference is not that
"IntelTdx.dsc"
contain the *complete* TDVF feature set. The relevant part (for me)
is that
"OvmfPkgX64.dsc" *not* be over-complicated for the sake of TDX,
even
considering only the "basic" TDVF feature set. It's fine to
implement TDX in two
stages ("basic" and "complete"); my point is that even "basic"
should not over-
complicate "OvmfPkgX64.dsc".
Thanks much for the comments and we don't want to make OvmfPkgX64.dsc
over-complicated either.
We have updated the design slides to V0.95 and Slides 6-15 are
discussing the
Config-A and Config-B.
https://edk2.groups.io/g/devel/files/Designs/2021/0611/TDVF_Design_Review%28v0.95%29.pptx
Your comment is always welcome!
The mailing list still won't give me that file, can you update it in
the bugzilla:

https://bugzilla.tianocore.org/show_bug.cgi?id=3429

As well, please?

Thanks,

James


Re: [edk2-devel] RFC: design review for TDVF in OVMF

Min Xu <min.m.xu@...>
 

On 06/22/2021 9:39 PM, Laszlo wrote:

I should clarify: the relevant part of my preference is not that "IntelTdx.dsc"
contain the *complete* TDVF feature set. The relevant part (for me) is that
"OvmfPkgX64.dsc" *not* be over-complicated for the sake of TDX, even
considering only the "basic" TDVF feature set. It's fine to implement TDX in two
stages ("basic" and "complete"); my point is that even "basic" should not over-
complicate "OvmfPkgX64.dsc".
Thanks much for the comments and we don't want to make OvmfPkgX64.dsc
over-complicated either.
We have updated the design slides to V0.95 and Slides 6-15 are discussing the
Config-A and Config-B.
https://edk2.groups.io/g/devel/files/Designs/2021/0611/TDVF_Design_Review%28v0.95%29.pptx
Your comment is always welcome!

Thanks!
Min


Re: [edk2-devel] RFC: design review for TDVF in OVMF

Laszlo Ersek
 

On 06/23/21 04:44, Xu, Min M wrote:
On 06/22/2021 9:35 PM, Laszlo wrote:

For example, as I stated earlier, "OvmfPkg/AcpiPlatformDxe" is a
driver where I'd like to see zero changes, for either SEV or TDX. If
the TD Mailbox location has to be reported to the OS via the MADT,
and QEMU cannot (or must not) populate that field in the MADT, then a
separate, TDX-specific edk2 driver should locate the MADT (installed
technically by "OvmfPkg/AcpiPlatformDxe", earlier), and update the
field.
We have updated the design of AcpiPlatformDxe. Please see the slides
in below link.
https://edk2.groups.io/g/devel/files/Designs/2021/0611/TDVF_Design_Review-AcpiPlatformDxe.pptx
Thanks, let me mark this with [1].


Because MailboxAddress in MADT table is determined in runtime in Tdx,
so we separate the update of the MADT table in TdxDxe driver and keep
AcpiPlatformDxe clean and shim.
I've now read

4.3.4 AP information reporting from TDVF to OS

from

https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf

That section does not go into much detail about the expected MADT
updates / entries.

I've also checked the various MADT subtable types here:

https://uefi.org/specs/ACPI/6.4/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#interrupt-controller-structure-types

It seems that the TDVF spec speaks about subtable type 0 (Processor
Local APIC):

https://uefi.org/specs/ACPI/6.4/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#processor-local-apic-structure

I've also checked "acpidump -b; iasl -d" in a normal guest, to remind
myself of the actual MADT contents that QEMU currently generates. I see
minimally the following subtable types:

Subtable Type : 00 [Processor Local APIC]
Subtable Type : 01 [I/O APIC]
Subtable Type : 02 [Interrupt Source Override]
Subtable Type : 04 [Local APIC NMI]

Thus, the TDVF spec creates the extremely unfortunate situation where
subtables of types different from 0 are expected from QEMU, but
subtables of type 0 are expected from the firmware.

In this case, QEMU should likely not populate the MADT with any LAPIC (=
type 0) subtables. Then, Option-3 from slide#5 in [1] (uninstalling the
MADT, *extending* the MADT with LAPIC subtables, installing the new
MADT) seems relatively workable.

(

Note that uninstalling an ACPI table (with EFI_ACPI_TABLE_PROTOCOL)
that was installed by a different driver previously requires the use
of EFI_ACPI_SDT_PROTOCOL. That's because the TableKey parameter taken
by EFI_ACPI_TABLE_PROTOCOL.UninstallAcpiTable() is only available from
EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() -- which the TDX driver
will not have called --, or from EFI_ACPI_SDT_PROTOCOL.GetAcpiTable().

EFI_ACPI_SDT_PROTOCOL.RegisterNotify() also exists, but it should be
avoided. It should not be used to immediately modify or replace the
MADT as soon as the MADT appears. That's because, upon encountering an
error, OvmfPkg/AcpiPlatformDxe rolls back all tables it installed up
to the error. It wouldn't be great if the rollback attempted to remove
a different MADT than what was installed.

)


Another approach (which sounds quite horrible) might be for QEMU to
pre-populate the MADT with just the right *count* of LAPIC subtables,
and then the TDX driver would patch the MADT *in-place* with the proper
LAPIC subtable contents (and then re-checksum the table manually). This
sounds horrible indeed.


Modifying the InstallAcpiTables() function in OvmfPkg/AcpiPlatformDxe,
so that it install a custom NULL protocol when InstallQemuFwCfgTables()
succeeds, seems tolerable. (In order to trigger the TdxDxe driver's MADT
patching.) However, I don't understand the following comment from
slide#5:

Open: This method is not practicable if parameters cannot be
transferred when trigger the notify function.
What parameters are we talking about here? The TDVF design guide speaks
about TD_VCPU_INFO_HOB, regarding the information required for filling
in the LAPIC entries in the MADT. But the same HOB should be available
to the TDX DXE driver too.


It would be much better if TDX specific code were added to QEMU that
prevented the generation of the MADT altogether, when running a TDX
guest. Then the firmware would fully own the MADT, and the TDX DXE
driver would only have to wait for the availability of
EFI_ACPI_TABLE_PROTOCOL (simple depex). In this case, of course, the TDX
driver would be responsible for all other subtable types too, not just
type 0 (LAPIC).


If all else fails, you can also copy "OvmfPkg/AcpiPlatformDxe" to
"OvmfPkg/TdxAcpiPlatformDxe", and customize it in any way (e.g. as
described on slides #3 and #4 [1]; Options 1 and 2). In this case,
TdxAcpiPlatformDxe would only be used in "IntelTdx.dsc", not the
pre-existent OvmfPkg*.dsc files.


Summary:

- Options 1 and 2 from [1] are not acceptable for
OvmfPkg/AcpiPlatformDxe.

- Option 3 from [1] is acceptable for OvmfPkg/AcpiPlatformDxe with a
custom NULL protocol instance, but:

- I don't understand the "parameter passing problem".

- How exactly the TdxDxe driver implements the MADT update (or
replacement) remains a question, impacting even QEMU (as QEMU and
TdxDxe must not fight for ownership over the LAPIC subtables). Some
possibilities are:

- stop QEMU from generating the MADT, make TdxDxe own the MADT fully;

- stop QEMU from generating LAPIC subtables;

- make QEMU generate the right number of dummy MADT entries;

- don't touch QEMU, but filter out its LAPIC subtables in TdxDxe.

- Options 1 and 2 from [1] are acceptable for a detached / distinct
driver called "OvmfPkg/TdxAcpiPlatformDxe", but this driver could only
be used in "IntelTdx.dsc".

Thanks
Laszlo


Re: [edk2-devel] RFC: design review for TDVF in OVMF

Min Xu <min.m.xu@...>
 

On 06/22/2021 9:35 PM, Laszlo wrote:
Hi,

On 06/11/21 08:37, Xu, Min M wrote:
In today's TianoCore Design Meeting we reviewed the Overview Section
(from slide 1 to 20). Thanks much for the valuable feedbacks and comments.
The meeting minutes will be sent out soon.

To address the concerns of the *one binary* solution in previous
discussion, we propose 2 Configurations for TDVF to upstream. (slide 6
- 8)



Config-A:

* Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align
with existing SEV)
* Threat model: VMM is NOT out of TCB. (We don't make things worse.)
* The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic boot
capability. The final binary can run on SEV/TDX/normal OVMF
* No changes to existing OvmfPkgX64 image layout.
* No need to add additional security features if they do not exist today
* No need to remove features if they exist today.
* RTMR is not supported
* PEI phase is NOT skipped in either Td or Non-Td
(so this is "Config-A / Option B", per slide 9 in the v0.9 slide deck)
Yes, in Config-A we chose to follow the standard EDK2 flow (SEC -> PEI -> DXE -> BDS)
So that the changes in Config-A is not too intrusive.





Config-B:

* Add a standalone IntelTdx.dsc to a TDX specific directory for a *full*
feature TDVF. (Align with existing SEV)
* Threat model: VMM is out of TCB. (We need necessary change to
prevent attack from VMM)
* IntelTdx.dsc includes TDX/normal OVMF basic boot capability. The final
binary can run on TDX/normal OVMF
* It might eventually merge with AmdSev.dsc, but NOT at this point of
time. And we don't know when it will happen. We need sync with AMD in
the community, after both of us think the solutions are mature to merge.
* Need to add necessary security feature as mandatory requirement,
such as RTMR based Trusted Boot support
* Need to remove unnecessary attack surfaces, such as network stack.
After reading the above, and checking slides 6 through 10 of the v0.9 slide
deck:

- I prefer Config-B (IntelTdx.dsc).

This is in accordance with what I wrote earlier about "OvmfPkgX64.dsc"
maintainability and regressions.

Additionally (given that a full-featured TDVF is the ultimate goal), I see the
advance from "Config-A / option B" to "Config-B" a lot less
*incremental* than the step from "OvmfPkgX64.dsc" to "AmdSev.dsc" was.

Put differently, I think that any TDX work targeted at "OvmfPkgX64.dsc"
is going to prove less useful for the final "IntelTdx.dsc" than how reusable
SEV work from "OvmfPkgX64.dsc" did for "AmdSev.dsc".

Put yet differently, I'm concerned that a part of the TDX work for
"OvmfPkgX64.dsc" might be a waste, with an eye towards the ultimate TDVF
feature set ("IntelTdx.dsc").
Actually Config-A and Config-B share some common (or basic) TDX features,
for example, the ResetVector, Memory Accept in SEC phase, IoMMU/DMA in
DXE phase, and the base IoLib, etc.
Config-A supports the basic Tdx features (except the security features).
Config-B supports the full set of Tdx features.


- I could (very cautiously) live with "Config-A / option B" as the initial
approach. However, we'de have to be ready to make the full split (the
switch-over to "IntelTdx.dsc") at *any point* during development, in case
something turns out to be too intrusive. (And yes, "too intrusive" is
subjective.)
Yes, we will always keep in mind the maintainability and regressions about
"OvmfPkgX64.dsc". So as the initial approach, only the basic Tdx features will
be included in Config-A.

By this I mean that any particular patch towards "Config-A / option B"
could cause me to ask, "please create IntelTdx.dsc now". Note that the later
we make the switch the more painful it could be (= the more invested in
"OvmfPkgX64.dsc" we could be, at that point).
Yes we will submit the patch for Config-B when any particular patch towards
"Config-A", so that we will not have a big surprise in the future.
Thanks!
Min

1 - 20 of 694