Re: [PATCH v3 1/2] fs/erofs: Add support for EROFS

[Gao Xiang <hsiangkao@linux.alibaba.com>]

Hi Yifan,

On 2023/7/21 11:29, Yifan Zhao wrote:
> EROFS is a lightweight read-only filesystem designed for performance which
> has already been widely used in several scenarios, such as Android system
> partitions, container images, and rootfs for embedded devices.
> 
> This patch brings EROFS uncompressed support. Now, it's possible to boot
> directly through GRUB with an EROFS rootfs.
> 
> EROFS compressed support will be developed later since it has more work
> to polish.
> 

It'd be better to collect all previous RVB tags here for others to refer.

> Signed-off-by: Yifan Zhao <zhaoyifan@sjtu.edu.cn>
> ---

...

> diff --git a/INSTALL b/INSTALL
> index b93fe9c61..1939e4745 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -77,15 +77,15 @@ Prerequisites for make-check:
>   
>   * If running a Linux kernel the following modules must be loaded:
>     - fuse, loop
> -  - btrfs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, nilfs2,
> +  - btrfs, erofs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, nilfs2,
>       reiserfs, udf, xfs
>     - On newer kernels, the exfat kernel modules may be used instead of the
>       exfat FUSE filesystem
>   * The following are Debian named packages required mostly for the full
>     suite of filesystem testing (but some are needed by other tests as well):
> -  - btrfs-progs, dosfstools, e2fsprogs, exfat-utils, f2fs-tools, genromfs,
> -    hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs, squashfs-tools,
> -    reiserfsprogs, udftools, xfsprogs, zfs-fuse
> +  - btrfs-progs, dosfstools, erofs_utils, e2fsprogs, exfat-utils, f2fs-tools,

There is no erofs_utils, please use erofs-utils here.

> +    genromfs, hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs,
> +    squashfs-tools, reiserfsprogs, udftools, xfsprogs, zfs-fuse
>     - exfat-fuse, if not using the exfat kernel module
>     - gzip, lzop, xz-utils
>     - attr, cpio, g++, gawk, parted, recode, tar, util-linux
> diff --git a/Makefile.util.def b/Makefile.util.def
> index 1e9a13d3e..ca2a32a7f 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -98,6 +98,7 @@ library = {
>     common = grub-core/fs/cpio_be.c;
>     common = grub-core/fs/odc.c;
>     common = grub-core/fs/newc.c;
> +  common = grub-core/fs/erofs.c;
>     common = grub-core/fs/ext2.c;
>     common = grub-core/fs/fat.c;
>     common = grub-core/fs/exfat.c;
> diff --git a/docs/grub.texi b/docs/grub.texi
> index b81267980..60c98e931 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -361,6 +361,7 @@ blocklist notation. The currently supported filesystem types are @dfn{Amiga
>   Fast FileSystem (AFFS)}, @dfn{AtheOS fs}, @dfn{BeFS},
>   @dfn{BtrFS} (including raid0, raid1, raid10, gzip and lzo),
>   @dfn{cpio} (little- and big-endian bin, odc and newc variants),
> +@dfn{EROFS} (only uncompressed support for now),
>   @dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32},
>   @dfn{exFAT}, @dfn{F2FS}, @dfn{HFS}, @dfn{HFS+},
>   @dfn{ISO9660} (including Joliet, Rock-ridge and multi-chunk files),
> @@ -6212,7 +6213,7 @@ assumed to be encoded in UTF-8.
>   NTFS, JFS, UDF, HFS+, exFAT, long filenames in FAT, Joliet part of
>   ISO9660 are treated as UTF-16 as per specification. AFS and BFS are read
>   as UTF-8, again according to specification. BtrFS, cpio, tar, squash4, minix,
> -minix2, minix3, ROMFS, ReiserFS, XFS, ext2, ext3, ext4, FAT (short names),
> +minix2, minix3, ROMFS, ReiserFS, XFS, EROFS, ext2, ext3, ext4, FAT (short names),
>   F2FS, RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are assumed
>   to be UTF-8. This might be false on systems configured with legacy charset
>   but as long as the charset used is superset of ASCII you should be able to
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index d2cf29584..eccf9b6cf 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1438,6 +1438,11 @@ module = {
>     common = fs/odc.c;
>   };
>   
> +module = {
> +  name = erofs;
> +  common = fs/erofs.c;
> +};
> +
>   module = {
>     name = ext2;
>     common = fs/ext2.c;
> diff --git a/grub-core/fs/erofs.c b/grub-core/fs/erofs.c
> new file mode 100644
> index 000000000..1d44862b8
> --- /dev/null
> +++ b/grub-core/fs/erofs.c
> @@ -0,0 +1,974 @@
> +/* erofs.c - Enhanced Read-Only File System */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2023 Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/disk.h>
> +#include <grub/dl.h>
> +#include <grub/err.h>
> +#include <grub/file.h>
> +#include <grub/fs.h>
> +#include <grub/fshelp.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/safemath.h>
> +#include <grub/types.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#define EROFS_SUPER_OFFSET (1024)
> +#define EROFS_MAGIC 0xE0F5E1E2
> +#define EROFS_ISLOTBITS (5)
> +
> +#define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004
> +#define EROFS_ALL_FEATURE_INCOMPAT (EROFS_FEATURE_INCOMPAT_CHUNKED_FILE)
> +
> +struct grub_erofs_super
> +{
> +  grub_uint32_t magic;
> +  grub_uint32_t checksum;
> +  grub_uint32_t feature_compat;
> +  grub_uint8_t log2_blksz;
> +  grub_uint8_t sb_extslots;
> +
> +  grub_uint16_t root_nid;
> +  grub_uint64_t inos;
> +
> +  grub_uint64_t build_time;
> +  grub_uint32_t build_time_nsec;
> +  grub_uint32_t blocks;
> +  grub_uint32_t meta_blkaddr;
> +  grub_uint32_t xattr_blkaddr;
> +  grub_uint8_t uuid[16];
> +  grub_uint8_t volume_name[16];
> +  grub_uint32_t feature_incompat;
> +
> +  union
> +  {
> +    grub_uint16_t available_compr_algs;
> +    grub_uint16_t lz4_max_distance;
> +  } GRUB_PACKED u1;
> +
> +  grub_uint16_t extra_devices;
> +  grub_uint16_t devt_slotoff;
> +  grub_uint8_t log2_dirblksz;
> +  grub_uint8_t xattr_prefix_count;
> +  grub_uint32_t xattr_prefix_start;
> +  grub_uint64_t packed_nid;
> +  grub_uint8_t reserved2[24];
> +} GRUB_PACKED;
> +
> +#define EROFS_INODE_LAYOUT_COMPACT 0
> +#define EROFS_INODE_LAYOUT_EXTENDED 1
> +
> +enum
> +{
> +  EROFS_INODE_FLAT_PLAIN = 0,
> +  EROFS_INODE_COMPRESSED_FULL = 1,
> +  EROFS_INODE_FLAT_INLINE = 2,
> +  EROFS_INODE_COMPRESSED_COMPACT = 3,
> +  EROFS_INODE_CHUNK_BASED = 4,
> +  EROFS_INODE_DATALAYOUT_MAX
> +};
> +
> +#define EROFS_I_VERSION_MASKS 0x01
> +#define EROFS_I_DATALAYOUT_MASKS 0x07
> +
> +#define EROFS_I_VERSION_BIT 0
> +#define EROFS_I_DATALAYOUT_BIT 1
> +
> +struct grub_erofs_inode_chunk_info
> +{
> +  grub_uint16_t format;
> +  grub_uint16_t reserved;
> +} GRUB_PACKED;
> +
> +#define EROFS_CHUNK_FORMAT_BLKBITS_MASK 0x001F
> +#define EROFS_CHUNK_FORMAT_INDEXES 0x0020
> +
> +#define EROFS_BLOCK_MAP_ENTRY_SIZE 4
> +
> +#define EROFS_NULL_ADDR -1
> +
> +struct grub_erofs_inode_chunk_index
> +{
> +  grub_uint16_t advise;
> +  grub_uint16_t device_id;
> +  grub_uint32_t blkaddr;
> +};
> +
> +union grub_erofs_inode_i_u
> +{
> +  grub_uint32_t compressed_blocks;
> +  grub_uint32_t raw_blkaddr;
> +
> +  grub_uint32_t rdev;
> +
> +  struct grub_erofs_inode_chunk_info c;
> +};
> +
> +struct grub_erofs_inode_compact
> +{
> +  grub_uint16_t i_format;
> +
> +  grub_uint16_t i_xattr_icount;
> +  grub_uint16_t i_mode;
> +  grub_uint16_t i_nlink;
> +  grub_uint32_t i_size;
> +  grub_uint32_t i_reserved;
> +
> +  union grub_erofs_inode_i_u i_u;
> +
> +  grub_uint32_t i_ino;
> +  grub_uint16_t i_uid;
> +  grub_uint16_t i_gid;
> +  grub_uint32_t i_reserved2;
> +} GRUB_PACKED;
> +
> +struct grub_erofs_inode_extended
> +{
> +  grub_uint16_t i_format;
> +
> +  grub_uint16_t i_xattr_icount;
> +  grub_uint16_t i_mode;
> +  grub_uint16_t i_reserved;
> +  grub_uint64_t i_size;
> +
> +  union grub_erofs_inode_i_u i_u;
> +
> +  grub_uint32_t i_ino;
> +
> +  grub_uint32_t i_uid;
> +  grub_uint32_t i_gid;
> +  grub_uint64_t i_mtime;
> +  grub_uint32_t i_mtime_nsec;
> +  grub_uint32_t i_nlink;
> +  grub_uint8_t i_reserved2[16];
> +} GRUB_PACKED;
> +
> +enum
> +{
> +  EROFS_FT_UNKNOWN,
> +  EROFS_FT_REG_FILE,
> +  EROFS_FT_DIR,
> +  EROFS_FT_CHRDEV,
> +  EROFS_FT_BLKDEV,
> +  EROFS_FT_FIFO,
> +  EROFS_FT_SOCK,
> +  EROFS_FT_SYMLINK,
> +  EROFS_FT_MAX
> +};
> +
> +#define EROFS_NAME_LEN 255
> +
> +struct grub_erofs_dirent
> +{
> +  grub_uint64_t nid;
> +  grub_uint16_t nameoff;
> +  grub_uint8_t file_type;
> +  grub_uint8_t reserved;
> +} GRUB_PACKED;
> +
> +enum
> +{
> +  BH_Meta,
> +  BH_Mapped,
> +};

Why need those (BH_xxx) for a bootloader? you could always use

#define EROFS_MAP_META		0x01
#define EROFS_MAP_MAPPED	0x02

Otherwise it looks good to me too, please fix the nits above:
Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang