This series is intended to restart the conversation re. adding
boot support for Mac OS X to QEMU via OVMF.

The *last* three patches represent what's left of Reza's excellent
work on getting OVMF to support booting OS X. They are rebased to
the point where they still apply cleanly and work, but I haven't
made any significant changes to them beyond that.

The *first* three patches (1-3 of 6) provide the BSD-licensed HFS+
filesystem support required by Reza's work:

    - patch 1/6: After a bit of archaeology, I managed to isolate
                 the original EFI "File System Wrapper" (FSW) files
                 released under the BSD by the "rEFIt" project; One
                 additional file, also BSD-licensed, was imported
                 from the "rEFInd" project.
                 The FSW is an abstraction layer that facilitates
                 the creation of a file system driver by providing
                 a set methods for mount/unmount/readdir/etc.

    - patch 2/6: fix a few compiler errors and other discrepancies to
                 facilitate integrating FSW into EDK2/OVMF.

    - patch 3/6: A BSD-licensed implementation of an FSW HFS+ driver.
                 Based on Apple's HFS+ specification (TN1150), this is
                 a minimalistic, bare-bones set of FSW methods capable
                 of locating and loading files from an HFS+ volume.
                 Lots of functionality (e.g. stat, readdir, hard/sym
                 links, or accessing files with more than 8 fragments)
                 is unimplemented at this time. I only implemented the
                 methods necessary to support loading Apple's boot.efi
                 and kernel.

                 For any extra features (such as stat, readdir, etc.)
                 I'd need to figure out how to navigate around on a
                 mounted volume (e.g. from the UEFI shell ?) to cause
                 the rest of the methods to be called, so I could test
                 whether they work or not. Particularly support for
                 fragmented files -- I'd have to create one somehow
                 (against OS X and HFS's best efforts to prevent it),
                 then cause it to be accessed from the UEFI shell to
                 test that whatever I'm implementing actually works.
                 Any "EDK2 filesystem developer's primer" on how to
                 do those things would be immensely helpful at this point.

Also, I would like some feedback and advice on where to go from here.

At this time, neither the FSW wrapper nor the HFS+ driver I wrote comply
with EDK2's coding standards. Before I start working on that, it would
be helpful to find out whether the FSW layer is of any interest to EDK2
as a way to facilitate adding support for arbitrary new filesystems.

If not, it might be worth factoring out the common bits and roll the
the whole thing up into a standalone HFS+ driver, which would be a
significantly different direction to go.

The series is also available on GitHub, in my "macboot" branch:

	https://github.com/gsomlo/edk2/tree/macboot

A set of instructions on how to get things working is available here:

	http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/

Thanks in advance for any feedback, advice, etc.
  --Gabriel

Gabriel L. Somlo (6):
  FswHfsPlus: add File System Wrapper (FSW) interface code
  FswHfsPlus: connect FSW code to EDK2, fix compile discrepancies
  FswHfsPlus: implement FSW driver for the HFS+ file system
  EdkCompatibilityPkg: allow ConsoleControl protocol to be used
  OvmfPkg: add Apple boot support
  OvmfPkg: enable AppleSupport library for Ovmf firmware

 EdkCompatibilityPkg/EdkCompatibilityPkg.dec        |    2 +
 OvmfPkg/FswHfsPlus/FswHfsPlus.inf                  |   57 ++
 OvmfPkg/FswHfsPlus/fsw_base.h                      |  158 +++
 OvmfPkg/FswHfsPlus/fsw_core.c                      |  929 +++++++++++++++++
 OvmfPkg/FswHfsPlus/fsw_core.h                      |  517 ++++++++++
 OvmfPkg/FswHfsPlus/fsw_efi.c                       | 1040 ++++++++++++++++++++
 OvmfPkg/FswHfsPlus/fsw_efi.h                       |  102 ++
 OvmfPkg/FswHfsPlus/fsw_efi_base.h                  |   81 ++
 OvmfPkg/FswHfsPlus/fsw_efi_edk2_base.h             |   76 ++
 OvmfPkg/FswHfsPlus/fsw_efi_lib.c                   |  129 +++
 OvmfPkg/FswHfsPlus/fsw_hfsplus.c                   |  535 ++++++++++
 OvmfPkg/FswHfsPlus/fsw_hfsplus.h                   |  238 +++++
 OvmfPkg/FswHfsPlus/fsw_lib.c                       |  294 ++++++
 OvmfPkg/FswHfsPlus/fsw_strfunc.h                   |  453 +++++++++
 OvmfPkg/Include/Library/AppleSupportLib.h          |   28 +
 OvmfPkg/Library/AppleSupportLib/AppleSupport.c     |  107 ++
 .../Library/AppleSupportLib/AppleSupportLib.inf    |   50 +
 OvmfPkg/Library/AppleSupportLib/Bds.c              |  151 +++
 OvmfPkg/Library/AppleSupportLib/Bds.h              |   21 +
 OvmfPkg/Library/AppleSupportLib/Common.h           |   24 +
 OvmfPkg/Library/AppleSupportLib/Console.c          |   86 ++
 OvmfPkg/Library/AppleSupportLib/Console.h          |   28 +
 OvmfPkg/Library/AppleSupportLib/Datahub.c          |  104 ++
 OvmfPkg/Library/AppleSupportLib/Datahub.h          |   32 +
 .../Library/PlatformBootManagerLib/BdsPlatform.c   |   10 +
 .../Library/PlatformBootManagerLib/BdsPlatform.h   |    1 +
 .../PlatformBootManagerLib.inf                     |    1 +
 OvmfPkg/OvmfPkgIa32.dsc                            |    8 +
 OvmfPkg/OvmfPkgIa32.fdf                            |    3 +
 OvmfPkg/OvmfPkgIa32X64.dsc                         |    8 +
 OvmfPkg/OvmfPkgIa32X64.fdf                         |    3 +
 OvmfPkg/OvmfPkgX64.dsc                             |    8 +
 OvmfPkg/OvmfPkgX64.fdf                             |    3 +
 33 files changed, 5287 insertions(+)
 create mode 100644 OvmfPkg/FswHfsPlus/FswHfsPlus.inf
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_base.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_core.c
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_core.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi.c
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_base.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_edk2_base.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_lib.c
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_hfsplus.c
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_hfsplus.h
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_lib.c
 create mode 100644 OvmfPkg/FswHfsPlus/fsw_strfunc.h
 create mode 100644 OvmfPkg/Include/Library/AppleSupportLib.h
 create mode 100644 OvmfPkg/Library/AppleSupportLib/AppleSupport.c
 create mode 100644 OvmfPkg/Library/AppleSupportLib/AppleSupportLib.inf
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Bds.c
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Bds.h
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Common.h
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Console.c
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Console.h
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Datahub.c
 create mode 100644 OvmfPkg/Library/AppleSupportLib/Datahub.h

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 03/07/17 04:14, Gabriel L. Somlo wrote:
> This series is intended to restart the conversation re. adding
> boot support for Mac OS X to QEMU via OVMF.

I have no way to test this, nor any interest to use it, so on the
community level, my only role here (for OvmfPkg) is to recommend a way
in which this (quite specific) feature can be accommodated without a
high risk of regressions and/or complexity increase. From reading the
commit messages, it looks like the OSX boot flow doesn't conform to UEFI
2.x, so any damage this feature does to OVMF (even in human
understanding) should be contained.

(1) Please introduce a new build flag, such as -D APPLE_BOOT_ENABLE or
-D OSX_BOOT_ENABLE, with a FALSE default value.

This build option should control:

(1a) whether FswHfsPlus and DataHubDxe are included in the DSC/FDF,

(1b) the value of a new FeaturePCD (also to be introduced). For the
name, I suggest gUefiOvmfPkgTokenSpaceGuid.PcdAppleBootEnable, or
...PcdOsxBootEnable.

In turn, the feature PCD should control code that's being added to
modules that we unconditionally build into OVMF. The prime examples for
this are the InitializeAppleSupport() and BdsBootApple() function calls
in PlatformBootManagerLib.

(1c) how the AppleSupportLib class is resolved. If the PCD / build flag
are false (the default), the lib class should be resolved to a Null
instance.

The point is, introducing a dependency on EdkCompatibilityPkg is
*incredibly* ugly, and if OVMF is built with the build flag disabled
(the default), I don't want that dependency to exist even at build time.
Hence the Null library instance, with no functionality and no dependencies.

(2) Please feed all the patches to:

  python BaseTools/Scripts/PatchCheck.py

because a number of coding style problems can be spotted. I'll comment
on those individually, as I notice them, but PatchCheck.py should help
in general.

Also please double-check if the new files are all CRLF terminated (I
didn't check this, just making sure).

(3) With the recent feature divergence in OvmfPkg (see also Xen PVH!),
it is high time we introduced maintainers / reviewers on the module (or
even file) level.

This would be a first for edk2, but in the mid-term, I'd strongly like
to assign Xen patch review and OSX boot patch review to dedicated
reviewers. No review from those reviewers --> no commit. The new
reviewers would not (immediately?) get push access, but their review
on-list would be of primary importance for pushing any patches.

Jordan, can you please help with figuring out the necessary format for
Maintainers.txt? If OSX boot has to be the feature that pioneers this,
so be it. In fact, such a detailed directory of maintainers would have
been useful for MdePkg and MdeModulePkg for years.

> The *last* three patches represent what's left of Reza's excellent
> work on getting OVMF to support booting OS X. They are rebased to
> the point where they still apply cleanly and work, but I haven't
> made any significant changes to them beyond that.
> 
> The *first* three patches (1-3 of 6) provide the BSD-licensed HFS+
> filesystem support required by Reza's work:

This is a filesystem driver which is not closely related to OVMF. It
should not go under OvmfPkg, but under a new top level package. I think
someone (maybe Jordan?) recommended FileSystemPkg/ earlier. Of course,
this would need a new entry in Maintainers.txt as well, and then, for
uniformity, FatPkg should be moved there as well.

> 
>     - patch 1/6: After a bit of archaeology, I managed to isolate
>                  the original EFI "File System Wrapper" (FSW) files
>                  released under the BSD by the "rEFIt" project; One
>                  additional file, also BSD-licensed, was imported
>                  from the "rEFInd" project.
>                  The FSW is an abstraction layer that facilitates
>                  the creation of a file system driver by providing
>                  a set methods for mount/unmount/readdir/etc.

So, technically speaking, since I've just suggested moving the driver
from under OvmfPkg to FileSystemPkg/, I no longer have any jurisdiction
over the driver. I will comment nonetheless, like any other contributor:

I disagree with the inclusion of any such external wrapper. In my mind
this is similar to how the Linux project rejects vendor-specific middle
layers. It might make sense for the rEFIt project to abstract the UEFI
interfaces even further, but having those one-off abstractions within
the edk2 tree does not make sense, in my opinion. Please code the driver
directly against UEFI interfaces. The reasoning is the same as the Linux
argument: anyone else looking at this code will expect to have to deal
with UEFI-level abstractions, and the filesystem spec, not another
(external) project's abstractions.

And, similarly to the AMD middle layer thing for Linux, the middle layer
being introduced here (in patch #1) is huge: the diffstat lists 3784
lines (the diffstat for the full series is 5287 lines). Please let's not
do this.

Again, this is just my opinion.

> 
>     - patch 2/6: fix a few compiler errors and other discrepancies to
>                  facilitate integrating FSW into EDK2/OVMF.
> 
>     - patch 3/6: A BSD-licensed implementation of an FSW HFS+ driver.
>                  Based on Apple's HFS+ specification (TN1150), this is
>                  a minimalistic, bare-bones set of FSW methods capable
>                  of locating and loading files from an HFS+ volume.
>                  Lots of functionality (e.g. stat, readdir, hard/sym
>                  links, or accessing files with more than 8 fragments)
>                  is unimplemented at this time.

This ties in nicely with my above argument. "stat", "readdir",
"hardlinks" and "symlinks" are concepts outside of the UEFI
specification, that is, outside of EFI_SIMPLE_FILE_SYSTEM_PROTOCOL and
EFI_FILE_PROTOCOL.

These primitives might make sense for rEFIt, but they make no sense for
a filesystem driver residing within the edk2 tree.

> I only implemented the
>                  methods necessary to support loading Apple's boot.efi
>                  and kernel.
> 
>                  For any extra features (such as stat, readdir, etc.)
>                  I'd need to figure out how to navigate around on a
>                  mounted volume (e.g. from the UEFI shell ?) to cause
>                  the rest of the methods to be called, so I could test
>                  whether they work or not. Particularly support for
>                  fragmented files -- I'd have to create one somehow
>                  (against OS X and HFS's best efforts to prevent it),
>                  then cause it to be accessed from the UEFI shell to
>                  test that whatever I'm implementing actually works.
>                  Any "EDK2 filesystem developer's primer" on how to
>                  do those things would be immensely helpful at this point.

If you implement a pure EFI_SIMPLE_FILE_SYSTEM_PROTOCOL and
EFI_FILE_PROTOCOL (with Revision==EFI_FILE_PROTOCOL_REVISION, that is,
not revision 2), you will see, from the UEFI spec, the member functions:
- Open(),
- Close(),
- Delete(),
- Read(),
- Write(),
- SetPosition(),
- GetPosition(),
- GetInfo(),
- SetInfo(),
- Flush().

Since this is (I assume?) a read-only driver, you can return
EFI_ACCESS_DENIED or EFI_WRITE_PROTECTED from Write(), SetInfo(), and
Flush(), as appropriate. Delete() should return EFI_WARN_DELETE_FAILURE.
Open() and Read() can be trivially tested in the UEFI shell (copy a file
to another, writeable (fat32) filesystem). SetPosition() and
GetPosition() can likely be implemented trivially. GetInfo() can be
tested with directory listing commands in the shell (dir, ls, AFAIR).

You can see a synthetic, read-only filesystem implementation in
"ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c".

> 
> Also, I would like some feedback and advice on where to go from here.
> 
> At this time, neither the FSW wrapper nor the HFS+ driver I wrote comply
> with EDK2's coding standards. Before I start working on that, it would
> be helpful to find out whether the FSW layer is of any interest to EDK2
> as a way to facilitate adding support for arbitrary new filesystems.

Ah, it's great that you ask this question explicitly! Please see my
answer above.

Regardless, there are some coding standard issues with the OvmfPkg
patches too (see my point (2) above); I shall comment on those elsewhere.

Thanks!
Laszlo

> 
> If not, it might be worth factoring out the common bits and roll the
> the whole thing up into a standalone HFS+ driver, which would be a
> significantly different direction to go.
> 
> The series is also available on GitHub, in my "macboot" branch:
> 
> 	https://github.com/gsomlo/edk2/tree/macboot
> 
> A set of instructions on how to get things working is available here:
> 
> 	http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/
> 
> Thanks in advance for any feedback, advice, etc.
>   --Gabriel
> 
> Gabriel L. Somlo (6):
>   FswHfsPlus: add File System Wrapper (FSW) interface code
>   FswHfsPlus: connect FSW code to EDK2, fix compile discrepancies
>   FswHfsPlus: implement FSW driver for the HFS+ file system
>   EdkCompatibilityPkg: allow ConsoleControl protocol to be used
>   OvmfPkg: add Apple boot support
>   OvmfPkg: enable AppleSupport library for Ovmf firmware
> 
>  EdkCompatibilityPkg/EdkCompatibilityPkg.dec        |    2 +
>  OvmfPkg/FswHfsPlus/FswHfsPlus.inf                  |   57 ++
>  OvmfPkg/FswHfsPlus/fsw_base.h                      |  158 +++
>  OvmfPkg/FswHfsPlus/fsw_core.c                      |  929 +++++++++++++++++
>  OvmfPkg/FswHfsPlus/fsw_core.h                      |  517 ++++++++++
>  OvmfPkg/FswHfsPlus/fsw_efi.c                       | 1040 ++++++++++++++++++++
>  OvmfPkg/FswHfsPlus/fsw_efi.h                       |  102 ++
>  OvmfPkg/FswHfsPlus/fsw_efi_base.h                  |   81 ++
>  OvmfPkg/FswHfsPlus/fsw_efi_edk2_base.h             |   76 ++
>  OvmfPkg/FswHfsPlus/fsw_efi_lib.c                   |  129 +++
>  OvmfPkg/FswHfsPlus/fsw_hfsplus.c                   |  535 ++++++++++
>  OvmfPkg/FswHfsPlus/fsw_hfsplus.h                   |  238 +++++
>  OvmfPkg/FswHfsPlus/fsw_lib.c                       |  294 ++++++
>  OvmfPkg/FswHfsPlus/fsw_strfunc.h                   |  453 +++++++++
>  OvmfPkg/Include/Library/AppleSupportLib.h          |   28 +
>  OvmfPkg/Library/AppleSupportLib/AppleSupport.c     |  107 ++
>  .../Library/AppleSupportLib/AppleSupportLib.inf    |   50 +
>  OvmfPkg/Library/AppleSupportLib/Bds.c              |  151 +++
>  OvmfPkg/Library/AppleSupportLib/Bds.h              |   21 +
>  OvmfPkg/Library/AppleSupportLib/Common.h           |   24 +
>  OvmfPkg/Library/AppleSupportLib/Console.c          |   86 ++
>  OvmfPkg/Library/AppleSupportLib/Console.h          |   28 +
>  OvmfPkg/Library/AppleSupportLib/Datahub.c          |  104 ++
>  OvmfPkg/Library/AppleSupportLib/Datahub.h          |   32 +
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   |   10 +
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |    1 +
>  .../PlatformBootManagerLib.inf                     |    1 +
>  OvmfPkg/OvmfPkgIa32.dsc                            |    8 +
>  OvmfPkg/OvmfPkgIa32.fdf                            |    3 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                         |    8 +
>  OvmfPkg/OvmfPkgIa32X64.fdf                         |    3 +
>  OvmfPkg/OvmfPkgX64.dsc                             |    8 +
>  OvmfPkg/OvmfPkgX64.fdf                             |    3 +
>  33 files changed, 5287 insertions(+)
>  create mode 100644 OvmfPkg/FswHfsPlus/FswHfsPlus.inf
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_base.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_core.c
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_core.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi.c
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_base.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_edk2_base.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_efi_lib.c
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_hfsplus.c
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_hfsplus.h
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_lib.c
>  create mode 100644 OvmfPkg/FswHfsPlus/fsw_strfunc.h
>  create mode 100644 OvmfPkg/Include/Library/AppleSupportLib.h
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/AppleSupport.c
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/AppleSupportLib.inf
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Bds.c
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Bds.h
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Common.h
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Console.c
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Console.h
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Datahub.c
>  create mode 100644 OvmfPkg/Library/AppleSupportLib/Datahub.h
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Hi Gabriel,

First off, thanks for going to the effort of building an HFS+ driver!
Due to travel, I've only just got around to checking these out, sorry.

Before I can try to help out with cleaning the patches up to the
extent they can be upstreamed, I still need to get them working.
boot.efi seems to load, and it finds the kernel, so the HFS+ driver is
definitely working, but it fails during early boot with "Error loading
drivers." This is with existing VM disk images that work with an old
variant of Reza's patchset, so it's possible that the prelinked kernel
image assumes something about the EFI implementation that's not true
for this patchset. I'll try reinstalling the VM from scratch with the
instructions from your website [1] before I try to dig deeper. It
could also be something to do with the hardlink issue I mention later
on, or maybe the lack of support for fragmented files.

A few general comments so far on the HFS patches below:

On Tue, Mar 7, 2017 at 4:14 PM, Gabriel L. Somlo <gsomlo@gmail.com> wrote:
>
>     - patch 3/6: A BSD-licensed implementation of an FSW HFS+ driver.
>                  Based on Apple's HFS+ specification (TN1150), this is
>                  a minimalistic, bare-bones set of FSW methods capable
>                  of locating and loading files from an HFS+ volume.
>                  Lots of functionality (e.g. stat, readdir, hard/sym
>                  links, or accessing files with more than 8 fragments)
>                  is unimplemented at this time. I only implemented the
>                  methods necessary to support loading Apple's boot.efi
>                  and kernel.

The OS X installer disk images produced by Apple's own
"createinstallmedia" tool creates hardlinks to the bootloader and
kernel image, so supporting hardlinks might be a useful feature we can
add, as it would simplify the installer image creation process
somewhat. But I guess the higher priority is getting rid of the FSW
wrapper.

> If not, it might be worth factoring out the common bits and roll the
> the whole thing up into a standalone HFS+ driver, which would be a
> significantly different direction to go.

Based on Laszlo's comments, that seems to be the preferred approach.
(Not entirely unexpectedly.) I'm unfamiliar with the FSW, so I'll try
to find some time to look over that and work out if I can make a
sufficient time investment to help out with distilling this down. Do
you think it'd be easier to start from zero, or to rip out FSW bits
one by one and hard-code them into the HFS+ driver, having the whole
thing working throughout?

Phil
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Hi Phil,

On Thu, Mar 30, 2017 at 12:22:16PM +1300, Phil Dennis-Jordan wrote:
> First off, thanks for going to the effort of building an HFS+ driver!
> Due to travel, I've only just got around to checking these out, sorry.
> 
> Before I can try to help out with cleaning the patches up to the
> extent they can be upstreamed, I still need to get them working.
> boot.efi seems to load, and it finds the kernel, so the HFS+ driver is
> definitely working, but it fails during early boot with "Error loading
> drivers." This is with existing VM disk images that work with an old
> variant of Reza's patchset, so it's possible that the prelinked kernel
> image assumes something about the EFI implementation that's not true
> for this patchset. I'll try reinstalling the VM from scratch with the
> instructions from your website [1] before I try to dig deeper. It
> could also be something to do with the hardlink issue I mention later
> on, or maybe the lack of support for fragmented files.
> 
> A few general comments so far on the HFS patches below:
> 
> On Tue, Mar 7, 2017 at 4:14 PM, Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> >
> >     - patch 3/6: A BSD-licensed implementation of an FSW HFS+ driver.
> >                  Based on Apple's HFS+ specification (TN1150), this is
> >                  a minimalistic, bare-bones set of FSW methods capable
> >                  of locating and loading files from an HFS+ volume.
> >                  Lots of functionality (e.g. stat, readdir, hard/sym
> >                  links, or accessing files with more than 8 fragments)
> >                  is unimplemented at this time. I only implemented the
> >                  methods necessary to support loading Apple's boot.efi
> >                  and kernel.
> 
> The OS X installer disk images produced by Apple's own
> "createinstallmedia" tool creates hardlinks to the bootloader and
> kernel image, so supporting hardlinks might be a useful feature we can
> add, as it would simplify the installer image creation process
> somewhat. But I guess the higher priority is getting rid of the FSW
> wrapper.
> 
> > If not, it might be worth factoring out the common bits and roll the
> > the whole thing up into a standalone HFS+ driver, which would be a
> > significantly different direction to go.
> 
> Based on Laszlo's comments, that seems to be the preferred approach.
> (Not entirely unexpectedly.) I'm unfamiliar with the FSW, so I'll try
> to find some time to look over that and work out if I can make a
> sufficient time investment to help out with distilling this down. Do
> you think it'd be easier to start from zero, or to rip out FSW bits
> one by one and hard-code them into the HFS+ driver, having the whole
> thing working throughout?

FTR, my main objectives with that patch set were, in roughly this
order:

	- backup: if I get hit by a bus, i wanted to have a (BSD)
	  license-compliant way to glue it all together and get it
	  working

	- something public to point at from my instructions webpage :)

	- solicit initial feedback re. HFS+ support
		- that's where Laszlo's "Lose the FSW!" came in :)

I decided to stick with FSW initially because I needed to focus on the
HFS+ nitty-gritty, and FSW allowed me to ignore the EDK2 nitty-gritty
during that process. (TBH, on the few occasions I've contributed anything
to EDK2, I always started out by writing in "regular" C coding style,
then translate into EDK2-ese immediately before submitting the patch :)

Last, but not least, I figured the refind project might also be
interested in a BSD-licensed HFS+ driver, so that'd be a free bonus.


If you decide to go for it and do the translation or
re-implementation, that'd be awesome! If it were me, I'd probably
try to delay the part where I eliminate the FSW until I were finished
with understanding and implementing the HFS+ specific bits. I'd delay
dealing with the coding guidelines until I had finished eliminating
the FSW, and had working C code I could understand just by
eyeballing...

But that's just me and my personal strength vs. weakness (mainly the
latter) tradeoff :) Any way that works for you is by definition better!


Last, but not least: I completely cargo-culted Reza's patches, with
only the bare minimum intervention to keep them compiling as I rebased
on top the accumulating upstream edk2 commits. Since they have no
reasonable chance of being applied without HFS+ support, I figured I'd
focus on that first and foremost. That also means that, right now, I have
absolutely no idea if (and how) anything in those patches could be done
more elegantly or efficently :)


Thanks again,
--Gabriel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel