From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A26AB803A6 for ; Fri, 10 Mar 2017 01:55:27 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Mar 2017 01:55:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,140,1486454400"; d="scan'208";a="66114780" Received: from mpvats-mobl2.amr.corp.intel.com (HELO localhost) ([10.254.184.25]) by orsmga004.jf.intel.com with ESMTP; 10 Mar 2017 01:55:26 -0800 MIME-Version: 1.0 To: Laszlo Ersek , edk2-devel-01 Message-ID: <148913972621.26490.6378343220939156912@jljusten-skl> From: Jordan Justen In-Reply-To: <3e3242e1-2578-3b14-636e-58d8000b41da@redhat.com> Cc: Ard Biesheuvel References: <20170223014814.10937-1-lersek@redhat.com> <148879199242.27619.7627489493363959501@jljusten-skl> <3e3242e1-2578-3b14-636e-58d8000b41da@redhat.com> User-Agent: alot/0.5.1 Date: Fri, 10 Mar 2017 01:55:26 -0800 Subject: Re: [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Mar 2017 09:55:27 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2017-03-06 10:41:31, Laszlo Ersek wrote: > On 03/06/17 10:19, Jordan Justen wrote: > = > > Is there any reason we couldn't add a > > pointer within the scatch buffer for reading/writing to? It just > > seems to make the interface a bit more generic/intuitive. > = > That's how I initially designed the interface, but while implementing > the functions, I realized there was no good use for that feature. > = > The only use case for storing the data read from fw_cfg at a nonzero > offset in the scratch buffer, and for writing data to fw_cfg from a > nonzero offset in the scratch buffer, would be that you have "something > else" at offset zero. What could that thing be? > = We discussed this on irc a bit. I agree that we can proceed with your current design. I still think the library interface would be clearer if it had the buffer passed to the read/write routines, but I concede that it would complicate and grow the code for no other gain than a clearer library interface. I think the helper callback adds to the confusion in the library interface, but if you don't have the buffer passed into the read/write routines, then you need the scratch buffer, and then the callback makes sense. I think we could have one helper to 'call me back when boot script is available', but didn't have the scratch allocation aspect. Of course, having torn out all the fw-cfg context, this now seems like something that some generic boot script lib might provide. Then I'd leave it up to the user of the lib to allocate a buffer and pass it into the fw-cfg boot script routines to be logged. The library would also have to allocate the DMA transfer buffer, which is just one way that this would be less efficient. Once again, I agree this would be less efficient, but I think it easier to follow. (Or, at least more intuitive from a library interface perspective.) And, once again, I am okay with you proceeding with your current design. -Jordan