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

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

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
> On Behalf Of Laszlo Ersek
> Sent: Monday, June 3, 2019 9:32 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/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,

Yes.

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

Yes.

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

The SerialDxe module uses the services of the SeriaPortLib to
produce the Serial I/O Protocol.  Instead of a physical register
interface such as UART in HW, the SW APIs of the SerialPortLib are
used.  The GUIDed Vendor HW node in the device path for the HW
register case would be a GUID value and name(s) that represents
the HW device used to perform the Serial I/O actions.  Applying
this to the SerialPortLib  access would imply GUID and Include
file names that are associated with the SerialPortLib. Perhaps:

  MdeModulePkg/Include/Guid/SerialPortLibVendor.h

  EDKII_SERIAL_PORT_LIB_VENDOR_GUID

  gEdkiiSerialPortLibVendorGuid

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