From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.5365.1685436674777111610 for ; Tue, 30 May 2023 01:51:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=GkZ5RDCr; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 2FD9B62275 for ; Tue, 30 May 2023 08:51:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 96756C433EF for ; Tue, 30 May 2023 08:51:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685436673; bh=L8svPYm4ZGO1O9dj5syj4yCkX6fSuyhZHPqYn/mihzk=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=GkZ5RDCrWMNvSC9RrKaW9Ab3/cMGS8EPJNk8lbAasCGhpwGeMPIicfLaXAKsFZdn7 lOLbMpybFAo8leaIYVQMs1QkMQQHQaaQ8S26NPMgrq7PfdAjwG+c4FoJyysbnziWt+ rxRA6AdXZor7tZI/K2bjNcQP7jp7IGu0+46wUJVJgtyOb1sBi5Viv82Aold6ZAWI7R QvYCHCEy9ZLUExJQJq1JqGROFwRTNJgx8yehm6s9AwAoy3Geqi7BdY6vJXt5DMOzQW iFdRufPIWoZXYNAYOZtsNGGwGsgwZBKyV5Rz8mbB2HBGih6GxV4vdBKV0XpBQZrJBK AT6UROH4u++/g== Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2af2696fd1cso43573371fa.2 for ; Tue, 30 May 2023 01:51:13 -0700 (PDT) X-Gm-Message-State: AC+VfDylO/n3JY5306ZoIQrD4VbkpLqF6BcQTVZ9MLeDoJWqR3SRgWf1 f2A717XXaGaMardw4wBr/eKkzIrsUr8HiVsiqq8= X-Google-Smtp-Source: ACHHUZ7BCT8vUA7nNsGX7yd6pOZrIWPXFPtBrSjnL91/O+4txEX/b4GOJvhJHg6KvNLAwYxnzUbYp3qahkFhrE6lkeI= X-Received: by 2002:a2e:2e16:0:b0:2ad:c1ec:fa3 with SMTP id u22-20020a2e2e16000000b002adc1ec0fa3mr521283lju.20.1685436671685; Tue, 30 May 2023 01:51:11 -0700 (PDT) MIME-Version: 1.0 References: <20230529101705.2476949-1-ardb@kernel.org> <20230529101705.2476949-6-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 30 May 2023 10:51:00 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use memory mapped FV protocol to avoid image copy To: "Ni, Ray" Cc: "devel@edk2.groups.io" , "Yao, Jiewen" , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , "Bi, Dandan" , "Gao, Liming" , "Kinney, Michael D" , Leif Lindholm , Michael Kubacki Content-Type: text/plain; charset="UTF-8" On Tue, 30 May 2023 at 10:41, Ni, Ray wrote: > > > > > -----Original Message----- > > From: Ard Biesheuvel > > Sent: Tuesday, May 30, 2023 3:52 PM > > To: devel@edk2.groups.io; Ni, Ray > > Cc: Yao, Jiewen ; Gerd Hoffmann ; > > Taylor Beebe ; Oliver Smith-Denny > denny.com>; Bi, Dandan ; Gao, Liming > > ; Kinney, Michael D ; > > Leif Lindholm ; Michael Kubacki > > > > Subject: Re: [edk2-devel] [RFC PATCH 05/11] MdeModulePkg/DxeCore: Use > > memory mapped FV protocol to avoid image copy > > > > On Tue, 30 May 2023 at 08:21, Ni, Ray wrote: > > > > > > GetFileBufferByFilePath() always returns a copy of file buffer even when the file > > > is in a memory-mapped device. > > > So, your patch adds a new implementation (abstracted through the new MM FV > > protocol) that can directly return the file location in the MMIO device. > > > > > > Several comments: > > > 1. I am not sure if any negative impact due to this change. For example: old > > logic reads the MMIO device but doesn't execute in the MMIO device. Does > > MMIO device always support execution in place? > > > > At this point, we are not executing anything in place. The buffer is > > only used by the PE/COFF loader to access the file contents, but it > > still creates the sections in memory as before, and copies the data > > into them. > > > > This is similar to how gBS->Loadimage() with a buffer/size only uses > > the buffer contents to access the file data, it does not execute the > > image from the buffer. > > OK. > > > > > > 2. If the MMFV protocol is only produced by DxeCore and consumed by > > DxeCore, can we just implement a local function instead? The challenge might be > > how to pass the FV_DEVICE instance to the local function. Can we "handle" the > > "FirmwareVolume2" protocol from the Handle first and use CR_FROM_THIS() > > macro to retrieve the FV_DEVICE pointer? This helps to not add the MMFV > > protocol, letting the change a pure DxeCore internal thing. > > > > > > > The loader does not know whether the FirmwareVolume2 protocol was > > produced by DXE core or by some other component, so we cannot assume > > that CR_FROM_THIS() is usable. > > I see. How about: > a. Define a GUID in DxeCore module internal > b. Install that GUID in the FV handle (the accordingly instance of that GUID can be simply NULL) > as a signature to tell the FV is produced by DxeCore > c. Implement a local function that return location inside the FV when the FvHandle has the > private GUID installed. > How is this any different from implementing a protocol? Installing a GUID on the handle and associating it with some code is exactly what this code does, but in the 'official' way. I'm not sure what we will gain by doing the same thing using local routines, other than making the code more difficult to follow. At least the protocol has a clearly defined interface, whereas the private GUID has no intrinsic meaning associated with it.