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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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.

Mike

> -----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
> >>
> >>
> >>
> >
> 
> 
>