From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web12.9430.1623329169541089261 for ; Thu, 10 Jun 2021 05:46:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=NdBr+2a8; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id C15EA61184 for ; Thu, 10 Jun 2021 12:46:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623329168; bh=B000yRHJV7mnbLQuyDPrK42hzsVsRibnTDmWPEFh1zA=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=NdBr+2a80+6IdT6aj3y1+CW/IO8ipER4vFlrNMk73ONAokEx+lpWt2GmnBdMhxdb7 qhafxioZ8yOZaCTLbJwg0nMuINIiCaLQ9/DNNH/nUa+aKYEDbaySCl35ubq+t0v+Nf GH8NemIWZVMrgMHXKX2jMR9rpSIXitRFfwEplHPOHQFbrzR1ZntCtdc3nzVBWokSUn JpqedsVYZ0iENYxM/BQ1VfewK6m7AO5T+DiKXeN5+7atFHxAT5kaCFEmN+IymQhgJR VpR0EK3Z/Stt/DiFGfov/I9L8MZ7BJBbofaKOoLA+DwtLNSI9RBfxr3e4tH1gYIZP3 lwepIMyjd1nkw== Received: by mail-oi1-f176.google.com with SMTP id u11so1985145oiv.1 for ; Thu, 10 Jun 2021 05:46:08 -0700 (PDT) X-Gm-Message-State: AOAM530f1DvAHAFlXjSP9FO8cR//WRzcG7nkaboq8mYx2Bps9KUgsaIS KCkEOz/LMXaPbf9gYr0vfbQDgPcxG/tBlWS+2GM= X-Google-Smtp-Source: ABdhPJwVIaIRRYkA1PqKEVtb67ZLX/NOtzWj4AVpzF950i0VeJ2l59iKllVf/LKaEufYdvx2VMucNap2LabytOwyszc= X-Received: by 2002:aca:eb55:: with SMTP id j82mr3203550oih.174.1623329166130; Thu, 10 Jun 2021 05:46:06 -0700 (PDT) MIME-Version: 1.0 References: <20210609121828.1884825-1-dovmurik@linux.ibm.com> <20210609121828.1884825-3-dovmurik@linux.ibm.com> In-Reply-To: <20210609121828.1884825-3-dovmurik@linux.ibm.com> From: "Ard Biesheuvel" Date: Thu, 10 Jun 2021 14:45:54 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH v1 2/3] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs To: edk2-devel-groups-io , Dov Murik Cc: Laszlo Ersek , Ard Biesheuvel , Jordan Justen , James Bottomley , Tobin Feldman-Fitzthum Content-Type: text/plain; charset="UTF-8" Hello Dov, On Wed, 9 Jun 2021 at 14:18, Dov Murik wrote: > > Remove the QemuFwCfgLib interface used to read the QEMU cmdline > (-append argument) and the initrd size. Instead, use the synthetic > filesystem QemuKernelLoaderFs which has three files: "kernel", "initrd", > and "cmdline". > > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: James Bottomley > Cc: Tobin Feldman-Fitzthum > Signed-off-by: Dov Murik > --- > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 2 +- > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 132 ++++++++++++++++++-- > 2 files changed, 126 insertions(+), 8 deletions(-) > > diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > index b262cb926a4d..f462fd6922cf 100644 > --- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > +++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > @@ -27,12 +27,12 @@ [LibraryClasses] > DebugLib > MemoryAllocationLib > PrintLib > - QemuFwCfgLib > UefiBootServicesTableLib > > [Protocols] > gEfiDevicePathProtocolGuid > gEfiLoadedImageProtocolGuid > + gEfiSimpleFileSystemProtocolGuid > > [Guids] > gQemuKernelLoaderFsMediaGuid > diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > index 114db7e8441f..7d26c912e14f 100644 > --- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > +++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > @@ -11,9 +11,9 @@ > #include > #include > #include > +#include > #include > #include > -#include > #include > #include > #include > @@ -30,6 +30,11 @@ typedef struct { > KERNEL_FILE_DEVPATH FileNode; > EFI_DEVICE_PATH_PROTOCOL EndNode; > } KERNEL_VENMEDIA_FILE_DEVPATH; > + > +typedef struct { > + VENDOR_DEVICE_PATH VenMediaNode; > + EFI_DEVICE_PATH_PROTOCOL EndNode; > +} SINGLE_VENMEDIA_NODE_DEVPATH; > #pragma pack () > > STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = { > @@ -51,6 +56,78 @@ STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = { > } > }; > > +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH mQemuKernelLoaderFileSystemDevicePath = { > + { > + { > + MEDIA_DEVICE_PATH, MEDIA_VENDOR_DP, > + { sizeof (VENDOR_DEVICE_PATH) } > + }, > + QEMU_KERNEL_LOADER_FS_MEDIA_GUID > + }, { > + END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, > + { sizeof (EFI_DEVICE_PATH_PROTOCOL) } > + } > +}; > + > +STATIC > +EFI_STATUS > +GetQemuKernelBlobSize ( Could we please use a better name here? 'QemuKernel' is confusing as it is used for initrd and cmdline only, in this case, right? 'QemuKernelLoader' is better in this regard, or other suggestions welcome as well. > + IN EFI_FILE_HANDLE Root, > + IN CHAR16 *FileName, > + OUT UINTN *Size > + ) > +{ > + EFI_STATUS Status; > + EFI_FILE_HANDLE FileHandle; > + UINT64 FileSize; > + > + Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Status = FileHandleGetSize (FileHandle, &FileSize); > + if (EFI_ERROR (Status)) { > + goto CloseFile; > + } > + *Size = FileSize; > + Status = EFI_SUCCESS; > +CloseFile: > + FileHandle->Close (FileHandle); > + return Status; > +} > + > +STATIC > +EFI_STATUS > +ReadWholeQemuKernelBlob ( > + IN EFI_FILE_HANDLE Root, > + IN CHAR16 *FileName, > + IN UINTN Size, > + OUT VOID *Buffer > + ) > +{ > + EFI_STATUS Status; > + EFI_FILE_HANDLE FileHandle; > + UINTN ReadSize; > + > + Status = Root->Open (Root, &FileHandle, FileName, EFI_FILE_MODE_READ, 0); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + ReadSize = Size; > + Status = FileHandle->Read (FileHandle, &ReadSize, Buffer); > + if (EFI_ERROR (Status)) { > + goto CloseFile; > + } > + if (ReadSize != Size) { > + Status = EFI_PROTOCOL_ERROR; > + goto CloseFile; > + } > + Status = EFI_SUCCESS; > +CloseFile: > + FileHandle->Close (FileHandle); > + return Status; > +} > + > /** > Download the kernel, the initial ramdisk, and the kernel command line from > QEMU's fw_cfg. The kernel will be instructed via its command line to load > @@ -79,6 +156,10 @@ QemuLoadKernelImage ( > EFI_STATUS Status; > EFI_HANDLE KernelImageHandle; > EFI_LOADED_IMAGE_PROTOCOL *KernelLoadedImage; > + EFI_DEVICE_PATH_PROTOCOL *DevicePathNode; > + EFI_HANDLE FsVolumeHandle; > + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *FsProtocol; Slightly tedious, but better to reindent the whole block. > + EFI_FILE_HANDLE Root; > UINTN CommandLineSize; > CHAR8 *CommandLine; > UINTN InitrdSize; > @@ -124,8 +205,38 @@ QemuLoadKernelImage ( > ); > ASSERT_EFI_ERROR (Status); > > - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineSize); > - CommandLineSize = (UINTN)QemuFwCfgRead32 (); > + // > + // Open the Qemu Kernel Loader abstract filesystem (volume) which will be > + // used to read the "initrd" and "cmdline" synthetic files. > + // > + DevicePathNode = (EFI_DEVICE_PATH_PROTOCOL *) &mQemuKernelLoaderFileSystemDevicePath; No space between cast type and castee please. > + Status = gBS->LocateDevicePath ( > + &gEfiSimpleFileSystemProtocolGuid, > + &DevicePathNode, > + &FsVolumeHandle > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status = gBS->HandleProtocol ( > + FsVolumeHandle, > + &gEfiSimpleFileSystemProtocolGuid, > + (VOID **)&FsProtocol > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status = FsProtocol->OpenVolume (FsVolumeHandle, &Root); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Status = GetQemuKernelBlobSize (Root, L"cmdline", &CommandLineSize); > + if (EFI_ERROR (Status)) { > + goto CloseRoot; > + } > > if (CommandLineSize == 0) { > KernelLoadedImage->LoadOptionsSize = 0; > @@ -136,8 +247,10 @@ QemuLoadKernelImage ( > goto UnloadImage; > } > > - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData); > - QemuFwCfgReadBytes (CommandLineSize, CommandLine); > + Status = ReadWholeQemuKernelBlob (Root, L"cmdline", CommandLineSize, CommandLine); > + if (EFI_ERROR (Status)) { > + goto FreeCommandLine; > + } > > // > // Verify NUL-termination of the command line. > @@ -155,8 +268,10 @@ QemuLoadKernelImage ( > KernelLoadedImage->LoadOptionsSize = (UINT32)((CommandLineSize - 1) * 2); > } > > - QemuFwCfgSelectItem (QemuFwCfgItemInitrdSize); > - InitrdSize = (UINTN)QemuFwCfgRead32 (); > + Status = GetQemuKernelBlobSize (Root, L"initrd", &InitrdSize); > + if (EFI_ERROR (Status)) { > + goto FreeCommandLine; > + } > > if (InitrdSize > 0) { > // > @@ -193,6 +308,7 @@ QemuLoadKernelImage ( > } > > *ImageHandle = KernelImageHandle; > + Root->Close (Root); > return EFI_SUCCESS; > > FreeCommandLine: > @@ -201,6 +317,8 @@ FreeCommandLine: > } > UnloadImage: > gBS->UnloadImage (KernelImageHandle); > +CloseRoot: > + Root->Close (Root); > > return Status; > } > -- > 2.25.1 > > > > ------------ > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#76267): https://edk2.groups.io/g/devel/message/76267 > Mute This Topic: https://groups.io/mt/83418869/5717338 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org] > ------------ > >