From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 03 Jun 2019 09:32:00 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4E24230C3198; Mon, 3 Jun 2019 16:31:59 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-13.rdu2.redhat.com [10.10.121.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id AF7CB60FD5; Mon, 3 Jun 2019 16:31:56 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/4] Define SERIAL_DXE_FILE_GUID only once To: "Kinney, Michael D" , "devel@edk2.groups.io" , "anthony.perard@citrix.com" Cc: "Wang, Jian J" , "Ni, Ray" , Ard Biesheuvel , "Zeng, Star" , "Wu, Hao A" , Julien Grall , Leif Lindholm References: <20190529113723.23186-1-anthony.perard@citrix.com> <972a37a8-196b-55de-41e6-13d4be044a50@redhat.com> From: "Laszlo Ersek" Message-ID: <2017a358-560a-0c32-00a0-e4dc041e22ac@redhat.com> Date: Mon, 3 Jun 2019 18:31:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Mon, 03 Jun 2019 16:31:59 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 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 ; >> devel@edk2.groups.io; anthony.perard@citrix.com >> Cc: Wang, Jian J ; Ni, Ray >> ; Ard Biesheuvel >> ; Zeng, Star >> ; Wu, Hao A ; >> Julien Grall ; Leif Lindholm >> >> 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 ; Ni, Ray >>>> ; Ard Biesheuvel >>>> ; Zeng, Star >>>> ; Wu, Hao A >> ; >>>> Julien Grall ; Leif Lindholm >>>> ; Laszlo Ersek >>>> ; Anthony PERARD >>>> >>>> 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 >>>> >>>> >>>> >>> >> >> >> >