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 05:02:03 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D22FA309266F; Mon, 3 Jun 2019 12:01:47 +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 3F305600CC; Mon, 3 Jun 2019 12:01:44 +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> From: "Laszlo Ersek" Message-ID: <972a37a8-196b-55de-41e6-13d4be044a50@redhat.com> Date: Mon, 3 Jun 2019 14:01:43 +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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Mon, 03 Jun 2019 12:01:53 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 >> >> >> >