Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once

["Laszlo Ersek" <lersek@redhat.com>]

On 06/03/19 16:50, Kinney, Michael D wrote:
> Laszlo,
> 
> We can keep compatibility.
> 
> The value of the GUID in the DEC/Include files can be the
> current SerialDxe FFS FILE_GUID.  Then the device path can
> use the GUID defined in the DEC/Include file and it will
> have the same value as today.
> 
> In the future, the SerialDxe FILE_GUID can be changed as
> needed with no impacts.
> 
> There is actually a build feature in the DSC file to
> override the FILE_GUID value of a single module in the
> module scoped <Defines> section.  If these feature was
> ever used with SerialDxe, then the generated device path
> would be wrong if EFI_CALLER_ID_GUID is used.

Right.

So, ultimately, we are asking Anthony to:

(1) please introduce the new GUID as EDKII_SERIAL_VENDOR_GUID and
gEdkiiSerialVendorGuid,

(2) please append another patch to this series, replacing
EFI_CALLER_ID_GUID -- and the comment! -- with EDKII_SERIAL_VENDOR_GUID,
in the "mSerialDevicePath" initializer (in
"MdeModulePkg/Universal/SerialDxe/SerialIo.c").

Correct?

Can we suggest an include file name too, in place of
"MdeModulePkg/Include/Guid/SerialDxe.h"?

Thanks!
Laszlo


>> -----Original Message-----
>> From: devel@edk2.groups.io
>> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
>> Sent: Monday, June 3, 2019 5:02 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>> devel@edk2.groups.io; anthony.perard@citrix.com
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
>> <ray.ni@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Zeng, Star
>> <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
>> <leif.lindholm@linaro.org>
>> Subject: Re: [edk2-devel] [PATCH 0/4] Define
>> SERIAL_DXE_FILE_GUID only once
>>
>> On 06/01/19 19:00, Kinney, Michael D wrote:
>>> Anthony,
>>>
>>> I am curious.  I agree that a GUID can be defined in
>> DEC file and
>>> in an include file that is used as a Vendor GUID in a
>> device path.
>>>
>>> Is there any reason that the FILE_GUID for the module
>> needs to be
>>> the same GUID value?  Is there any code that looks
>> for an FFS file
>>> with that GUID value as the FFS file name?
>>
>> It's the other way around.
>> "MdeModulePkg/Universal/SerialDxe" uses its own
>> FILE_GUID for populating a device path node on a handle
>> that it creates. Please see in "SerialIo.c":
>>
>> SERIAL_DEVICE_PATH mSerialDevicePath = {
>>   {
>>     { HARDWARE_DEVICE_PATH, HW_VENDOR_DP, { sizeof
>> (VENDOR_DEVICE_PATH), 0} },
>>     EFI_CALLER_ID_GUID  // Use the driver's GUID
>>   },
>>
>> In turn, PlatformBootManagerLib instances have to refer
>> to this device path (including said FILE_GUID) for
>> setting up their console UEFI variables.
>>
>>> If not, then it would be better to not over use that
>> GUID value.
>>> The FILE_GUID of the SerialDxe can be different.
>>
>> This would be a valid change, but for that, the device
>> path protocol installed by SerialDxe should be
>> decoupled from SerialDxe's own FILE_GUID.
>>
>> Dependent on platform circumstances, that could be a
>> compatibility issue, because the old ConIn / ConOut /
>> ErrOut variables could stop working (still referencing
>> the original FILE_GUID in the devpath).
>>
>> Thanks!
>> Laszlo
>>
>>>
>>> Thanks,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io
>>>> [mailto:devel@edk2.groups.io] On Behalf Of Anthony
>>>> PERARD
>>>> Sent: Wednesday, May 29, 2019 4:37 AM
>>>> To: devel@edk2.groups.io
>>>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray
>>>> <ray.ni@intel.com>; Ard Biesheuvel
>>>> <ard.biesheuvel@linaro.org>; Zeng, Star
>>>> <star.zeng@intel.com>; Wu, Hao A
>> <hao.a.wu@intel.com>;
>>>> Julien Grall <julien.grall@arm.com>; Leif Lindholm
>>>> <leif.lindholm@linaro.org>; Laszlo Ersek
>>>> <lersek@redhat.com>; Anthony PERARD
>>>> <anthony.perard@citrix.com>
>>>> Subject: [edk2-devel] [PATCH 0/4] Define
>>>> SERIAL_DXE_FILE_GUID only once
>>>>
>>>> The macro SERIAL_DXE_FILE_GUID is already been
>> defined
>>>> twice and the GUID is
>>>> been used once without defining the macro. This
>> patch
>>>> series define the macro
>>>> in MdeModulePkg where the SerialDxe is, and replace
>> all
>>>> other use by this new
>>>> one.
>>>>
>>>> Note that I haven't build/test those changes, but I
>>>> have test the first patch
>>>> by applying a similar change to a patch series I'm
>>>> working on.
>>>>
>>>> Patch series available in this git branch:
>>>> https://xenbits.xen.org/git-
>>>> http/people/aperard/ovmf.git br.serial-dxe-guid-v1
>>>>
>>>> Anthony PERARD (4):
>>>>   MdeModulePkg: Add SERIAL_DXE_FILE_GUID
>>>>   ArmVirtPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>   ArmPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>   UefiPayloadPkg/PlatformBootManagerLib: Use
>>>> SERIAL_DXE_FILE_GUID from
>>>>     MdeModulePkg
>>>>
>>>>  MdeModulePkg/MdeModulePkg.dec                 |  3
>> +++
>>>>  MdeModulePkg/Include/Guid/SerialDxe.h         | 19
>>>> +++++++++++++++++++
>>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
>> +--
>>>> ---
>>>>  .../PlatformBootManagerLib/PlatformBm.c       |  6
>> +--
>>>> ---
>>>>  .../PlatformBootManagerLib/PlatformConsole.c  |  3
>> ++-
>>>>  5 files changed, 26 insertions(+), 11 deletions(-)
>>>>  create mode 100644
>>>> MdeModulePkg/Include/Guid/SerialDxe.h
>>>>
>>>> --
>>>> Anthony PERARD
>>>>
>>>>
>>>>
>>>
>>
>>
>> 
>