Hey Alex, On Mon, May 27, 2024 at 9:51 PM Alexander Graf <g...@amazon.com> wrote: > > > On 27.05.24 16:52, Dorjoy Chowdhury wrote: > > Hi Philippe, > > Thank you for reviewing. > > > > On Mon, May 27, 2024 at 4:47 PM Philippe Mathieu-Daudé > > <phi...@linaro.org> wrote: > >> Hi Dorjoy, > >> > >> On 18/5/24 10:07, Dorjoy Chowdhury wrote: > >>> An EIF (Enclave Image Format)[1] image is used to boot an AWS nitro > >>> enclave[2] virtual machine. The EIF file contains the necessary > >>> kernel, cmdline, ramdisk(s) sections to boot. > >>> > >>> This commit adds support for loading EIF image using the microvm > >>> machine code. For microvm to boot from an EIF file, the kernel and > >>> ramdisk(s) are extracted into a temporary kernel and a temporary > >>> initrd file which are then hooked into the regular x86 boot mechanism > >>> along with the extracted cmdline. > >>> > >>> Although not useful for the microvm machine itself, this is needed > >>> as the following commit adds support for a new machine type > >>> 'nitro-enclave' which uses the microvm machine type as parent. The > >>> code for checking and loading EIF will be put inside a 'nitro-enclave' > >>> machine type check in the following commit so that microvm cannot load > >>> EIF because it shouldn't. > >>> > >>> [1] https://github.com/aws/aws-nitro-enclaves-image-format > >> The documentation is rather scarse... > >> > > Do you mean documentation about EIF file format? If so, yes, right > > now there is no specification other than the github repo for EIF. > > > >>> [2] https://aws.amazon.com/ec2/nitro/nitro-enclaves/ > >>> > >>> Signed-off-by: Dorjoy Chowdhury <dorjoychy...@gmail.com> > >>> --- > >>> hw/i386/eif.c | 486 ++++++++++++++++++++++++++++++++++++++++++++ > >>> hw/i386/eif.h | 20 ++ > >>> hw/i386/meson.build | 2 +- > >> ... still it seems a generic loader, not restricted to x86. > >> > >> Maybe better add it as hw/core/loader-eif.[ch]? > >> > > Yes, the code in eif.c is architecture agnostic. So it could make > > sense to move the files to hw/core. But I don't think the names should > > have "loader" prefix as there is no loading logic in eif.c. There is > > only logic for parsing and extracting kernel, intird, cmdline etc. > > Because nitro-enclave machine type is based on microvm which only > > supports x86 now, I think it also makes sense to keep the files inside > > hw/i386 for now as we can only really load x86 kernel using it. Maybe > > if we, in the future, add support for other architectures, then we can > > move them to hw/core. What do you think? > > > I think it makes sense to put EIF parsing into generic code from the > start. Nitro Enclaves supports Aarch64 with the same EIF semantics. In > fact, it would be pretty simple to do a sub-machine-class similar to the > x86 NE one for arm based on -M virt as a follow-up and by making the EIF > logic x86 only we're only making our lives harder for that future. >
Understood. I will move the eif.c and eif.h files to the hw/core directory. Thanks. Regards, Dorjoy
