From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.5980.1624959889290789740 for ; Tue, 29 Jun 2021 02:44:49 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=PsdIC4j2; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624959888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9RlFf1Y3zQxMIPuINJIkd9whkwUL/zGZRUILV/AKn3k=; b=PsdIC4j2HX00/BUYp1oug2VVaK0qxtAmNDifrc/dGBzztdknURHZhOoMOs8XTVFQq2QMfK Rq314tOjDxbzfWfw3WpfWiodHn1Jj7GXn1LVJWbMrMuzdVcJMEKdoQyI6lOadbCOujczme P/lQ2XSm9WrenJ/odabAKvZWUwZhX+A= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-236-Ln3ZJbVHOjqe7m0t7KTt1Q-1; Tue, 29 Jun 2021 05:44:46 -0400 X-MC-Unique: Ln3ZJbVHOjqe7m0t7KTt1Q-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B96AF8042A8; Tue, 29 Jun 2021 09:44:45 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-158.ams2.redhat.com [10.36.114.158]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EB9E118A77; Tue, 29 Jun 2021 09:44:43 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 4/5] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs To: devel@edk2.groups.io, dovmurik@linux.ibm.com Cc: Ard Biesheuvel , Jordan Justen , James Bottomley , Tobin Feldman-Fitzthum References: <20210628105110.379951-1-dovmurik@linux.ibm.com> <20210628105110.379951-5-dovmurik@linux.ibm.com> From: "Laszlo Ersek" Message-ID: Date: Tue, 29 Jun 2021 11:44:42 +0200 MIME-Version: 1.0 In-Reply-To: <20210628105110.379951-5-dovmurik@linux.ibm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 06/28/21 12:51, 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 > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3457 > Signed-off-by: Dov Murik > --- > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 3 +- > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 151 ++++++++++++++++++-- > 2 files changed, 139 insertions(+), 15 deletions(-) > > diff --git a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > index b262cb926a4d..9c9e35b1c5b9 100644 > --- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > +++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > @@ -25,14 +25,15 @@ [Packages] > > [LibraryClasses] > DebugLib > + FileHandleLib > 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 8a29976ae172..66e029397bd6 100644 > --- a/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > +++ b/OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c > @@ -11,13 +11,14 @@ > #include > #include > #include > +#include > #include > #include > -#include > #include > #include > #include > #include > +#include > > #pragma pack (1) > typedef struct { > @@ -30,6 +31,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 +57,82 @@ STATIC CONST KERNEL_VENMEDIA_FILE_DEVPATH mKernelDevicePath = { > } > }; > > +STATIC CONST SINGLE_VENMEDIA_NODE_DEVPATH mQemuKernelLoaderFsDevicePath = { > + { > + { > + 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 > +GetQemuKernelLoaderBlobSize ( > + 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; > + } > + if (FileSize > MAX_UINTN) { > + Status = EFI_UNSUPPORTED; > + goto CloseFile; > + } > + *Size = (UINTN)FileSize; > + Status = EFI_SUCCESS; > +CloseFile: > + FileHandle->Close (FileHandle); > + return Status; > +} > + > +STATIC > +EFI_STATUS > +ReadWholeQemuKernelLoaderBlob ( > + 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 > @@ -76,12 +158,16 @@ QemuLoadKernelImage ( > OUT EFI_HANDLE *ImageHandle > ) > { > - EFI_STATUS Status; > - EFI_HANDLE KernelImageHandle; > - EFI_LOADED_IMAGE_PROTOCOL *KernelLoadedImage; > - UINTN CommandLineSize; > - CHAR8 *CommandLine; > - UINTN InitrdSize; > + 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; > + EFI_FILE_HANDLE Root; > + UINTN CommandLineSize; > + CHAR8 *CommandLine; > + UINTN InitrdSize; > > // > // Load the image. This should call back into the QEMU EFI loader file system. > @@ -124,8 +210,38 @@ QemuLoadKernelImage ( > ); > ASSERT_EFI_ERROR (Status); > > - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineSize); > - CommandLineSize = (UINTN)QemuFwCfgRead32 (); > + // > + // Open the Qemu Kernel Loader abstract filesystem (volume) which will be > + // used to query the "initrd" and to read the "cmdline" synthetic files. > + // > + DevicePathNode = (EFI_DEVICE_PATH_PROTOCOL *)&mQemuKernelLoaderFsDevicePath; > + Status = gBS->LocateDevicePath ( > + &gEfiSimpleFileSystemProtocolGuid, > + &DevicePathNode, > + &FsVolumeHandle > + ); > + if (EFI_ERROR (Status)) { > + goto UnloadImage; > + } > + > + Status = gBS->HandleProtocol ( > + FsVolumeHandle, > + &gEfiSimpleFileSystemProtocolGuid, > + (VOID **)&FsProtocol > + ); > + if (EFI_ERROR (Status)) { > + goto UnloadImage; > + } > + > + Status = FsProtocol->OpenVolume (FsVolumeHandle, &Root); > + if (EFI_ERROR (Status)) { > + goto UnloadImage; > + } > + > + Status = GetQemuKernelLoaderBlobSize (Root, L"cmdline", &CommandLineSize); > + if (EFI_ERROR (Status)) { > + goto CloseRoot; > + } > > if (CommandLineSize == 0) { > KernelLoadedImage->LoadOptionsSize = 0; > @@ -133,11 +249,14 @@ QemuLoadKernelImage ( > CommandLine = AllocatePool (CommandLineSize); > if (CommandLine == NULL) { > Status = EFI_OUT_OF_RESOURCES; > - goto UnloadImage; > + goto CloseRoot; > } > > - QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData); > - QemuFwCfgReadBytes (CommandLineSize, CommandLine); > + Status = ReadWholeQemuKernelLoaderBlob (Root, L"cmdline", CommandLineSize, > + CommandLine); > + if (EFI_ERROR (Status)) { > + goto FreeCommandLine; > + } > > // > // Verify NUL-termination of the command line. > @@ -155,8 +274,10 @@ QemuLoadKernelImage ( > KernelLoadedImage->LoadOptionsSize = (UINT32)((CommandLineSize - 1) * 2); > } > > - QemuFwCfgSelectItem (QemuFwCfgItemInitrdSize); > - InitrdSize = (UINTN)QemuFwCfgRead32 (); > + Status = GetQemuKernelLoaderBlobSize (Root, L"initrd", &InitrdSize); > + if (EFI_ERROR (Status)) { > + goto FreeCommandLine; > + } > > if (InitrdSize > 0) { > // > @@ -199,6 +320,8 @@ FreeCommandLine: > if (CommandLineSize > 0) { > FreePool (CommandLine); > } > +CloseRoot: > + Root->Close (Root); > UnloadImage: > if (EFI_ERROR (Status)) { > gBS->UnloadImage (KernelImageHandle); > Reviewed-by: Laszlo Ersek