Re: [PATCH 00/12] ArmVirtPkg, OvmfPkg: factor out QemuFwCfgS3Lib

[Jordan Justen <jordan.l.justen@intel.com>]

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