Re: FmpDeviceLib

["Tomas Pilar (tpilar)" <tpilar@solarflare.com>]

Hi Mike,

Thanks for looking at this. I was actually thinking of how I could contribute. I was thinking of drafting the following patches:

1. Move FmpDxe data storage into heap allocated FMP_DATA struct, retrieve using standard CR against the protocol treatment. Allocated by FmpInstall(). This might need registering FmpRemove() however. Alternatively register a ExitBootServices callback that uninstalls the FMP.
2. Create FmpDeviceInstanceLib prototype that works like FmpDeviceLib but passes a context parameter to each function.
3. Implement FmpDeviceInstanceGlobal library that pulls in FmpDeviceLib and produces a FmpDeviceInstanceLib
4. Change over FmpDxe from FmpDeviceLib to depend on FmpDeviceInstanceLib.

This is not particularly urgent, but we are trying to implement FMP for our optionROM driver and it would be nice to be able to use the upstream cleanly rather than forking it in our driver.

If you think the way forward seems okay, I am happy to put in the work.

Cheers,
Tom

On 28/11/2018 01:15, Kinney, Michael D wrote:
> Tom,
>
> Thanks for noticing this issue.  I need a little 
> time to evaluate this use case and see what changes
> are required to make this work for a UEFI driver
> that manages many controllers.
>
> Thanks,
>
> Mike
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-
>> bounces@lists.01.org] On Behalf Of Tomas Pilar (tpilar)
>> Sent: Tuesday, November 27, 2018 7:29 AM
>> To: Gao, Liming <liming.gao@intel.com>; edk2-
>> devel@lists.01.org
>> Subject: Re: [edk2] FmpDeviceLib
>>
>> Okay, so I just noticed that FmpDxe is full of module
>> globals including the image descriptor, so it
>> necessarily requires that the driver that includes it as
>> a library can only ever control one controller.
>>
>> Cheers,
>> Tom
>>
>> On 27/11/2018 15:23, Tomas Pilar (tpilar) wrote:
>>> On 27/11/2018 14:33, Gao, Liming wrote:
>>>> Tom:
>>>>   FmpDeviceLib can use UEFI driver model/driver
>> binding protocol so it can install FMP on its device
>> handle during the BDS/Device connection phase. It can
>> implement RegisterFmpInstaller() to install FMP protocol
>> in its device handle. In this way, FmpDeviceLib is like
>> UEFI driver with UEFI driver binding protocol.
>>>> Thanks
>>>> Liming
>>> Hi Liming,
>>>
>>> Sure, so now I can install FMP onto my
>> ControllerHandle. Say that someone gets the FMP and
>> calls GetImageSize. The FmpDxeLib does some checks and
>> then it calls FmpDeviceLibGetImageSize() with no context
>> parameter. This method is supposed to be implemented by
>> me, the driver writer, but how is the code in this
>> method meant to know which Controller are we getting the
>> image size from?
>>> So maybe I can define some module globals, declare
>> them in a cross include file, include that in my driver
>> and and have the driver populate the module globals
>> during probe. This immediately limits the usefulness by
>> requiring that each driver only ever drives one
>> controller.
>>> And then you consider how to do a SetImage without
>> being even given the Handle of the Controller. Do you
>> stuff the controller handle into a module global
>> parameter that gets populated during the BDS phase?
>>> I guess you could enumerate all FMP instances, see
>> which one of them advertises your ImageTypeId and get
>> the handle that the FMP is installed on that way. But
>> that seems rather insane and would cause issues if you
>> have two of the same device in the platform, unless you
>> check the HardwareInstance as well? This seems insane.
>>> Cheers,
>>> Tom
>>>
>>>>> -----Original Message-----
>>>>> From: edk2-devel [mailto:edk2-devel-
>> bounces@lists.01.org] On Behalf Of Tomas Pilar (tpilar)
>>>>> Sent: Tuesday, November 27, 2018 9:26 PM
>>>>> To: edk2-devel@lists.01.org
>>>>> Subject: [edk2] FmpDeviceLib
>>>>>
>>>>> Hi all,
>>>>>
>>>>> I am looking at using FmpDxeLib so I need to
>> implement the FmpDeviceLib. However, it seems like the
>> library functions do not take any
>>>>> private struct as a parameter, so I am struggling to
>> figure out how to read information off the hardware.
>> FmpDxe does not even pass its
>>>>> created protocol instance when calling the library
>> functions, leading me to believe that the only way to do
>> this is to assign a pointer to
>>>>> private struct during library construction, but that
>> means that a driver that uses the code can only ever
>> service a single controller.
>>>>> Can you offer any insights?
>>>>>
>>>>> Cheers,
>>>>> Tom
>>>>> _______________________________________________
>>>>> edk2-devel mailing list
>>>>> edk2-devel@lists.01.org
>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel