| Message ID | CAFECyb-EMMQT1=XNNdY5ZwCK9OovzQ0TJT6bpQA1kofPwfdBZQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 14, 2015 at 1:57 PM, Kinney, Michael D <michael.d.kinney@intel.com> wrote: > Roy, > > 1) New GUID should be defined in MdeModulePkg/MdeModulePkg.dec > 2) New include file for new GUID in MdeModulePkg/Include/Guid > 3) Update TerminalDxe to use this new GUID. > > I do not recommend creating a new terminal driver. Please continue as you have which is adding support for this one new terminal type to MdeModulePkg/Universal/Console/TerminalDxe. > > Best regards, > > Mike > > OK, I have made the above changes, and have it working with one hack I'm not sure how to properly resolve. I now have gEfiTtyTermGuid declared (extern) in MdeModulePkg/Include/Guid/TtyTerm.h, and defined in MdeModulePkg/Universal/Console/TerminalDxe/Tty.c. I use gEfiTtyTermGuid in DevicePathFromText.c in MdePkg to convert between a text path to a device path, but I don't see how to include TtyTerm.h in DevicePathFromText.c. Right now I just have added the extern to DevicePathFromText.c to verify functionality. This works, but is obviously not the right solution. How should I go about properly getting gEfiTtyTermGuid declared in DevicePathFromText.c? Thanks, Roy ------------------------------------------------------------------------------
On 5 June 2015 at 06:03, Roy Franz <roy.franz@linaro.org> wrote: > On Thu, May 14, 2015 at 1:57 PM, Kinney, Michael D > <michael.d.kinney@intel.com> wrote: >> Roy, >> >> 1) New GUID should be defined in MdeModulePkg/MdeModulePkg.dec >> 2) New include file for new GUID in MdeModulePkg/Include/Guid >> 3) Update TerminalDxe to use this new GUID. >> >> I do not recommend creating a new terminal driver. Please continue as you have which is adding support for this one new terminal type to MdeModulePkg/Universal/Console/TerminalDxe. >> >> Best regards, >> >> Mike >> >> > OK, I have made the above changes, and have it working with one hack > I'm not sure how to properly resolve. > > I now have gEfiTtyTermGuid declared (extern) in > MdeModulePkg/Include/Guid/TtyTerm.h, and defined in > MdeModulePkg/Universal/Console/TerminalDxe/Tty.c. > > I use gEfiTtyTermGuid in DevicePathFromText.c in MdePkg to convert > between a text path to a device path, but > I don't see how to include TtyTerm.h in DevicePathFromText.c. Right > now I just have added the extern to DevicePathFromText.c > to verify functionality. This works, but is obviously not the right > solution. How should I go about properly getting gEfiTtyTermGuid > declared in DevicePathFromText.c? > You should not define the GUID yourself, you should add it to the appropriate .dec file, If you then depend on that package and guid in a module's .inf, the definition will be emitted implicitly by the build tools.
On Fri, Jun 5, 2015 at 2:31 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 5 June 2015 at 06:03, Roy Franz <roy.franz@linaro.org> wrote: >> On Thu, May 14, 2015 at 1:57 PM, Kinney, Michael D >> <michael.d.kinney@intel.com> wrote: >>> Roy, >>> >>> 1) New GUID should be defined in MdeModulePkg/MdeModulePkg.dec >>> 2) New include file for new GUID in MdeModulePkg/Include/Guid >>> 3) Update TerminalDxe to use this new GUID. >>> >>> I do not recommend creating a new terminal driver. Please continue as you have which is adding support for this one new terminal type to MdeModulePkg/Universal/Console/TerminalDxe. >>> >>> Best regards, >>> >>> Mike >>> >>> >> OK, I have made the above changes, and have it working with one hack >> I'm not sure how to properly resolve. >> >> I now have gEfiTtyTermGuid declared (extern) in >> MdeModulePkg/Include/Guid/TtyTerm.h, and defined in >> MdeModulePkg/Universal/Console/TerminalDxe/Tty.c. >> >> I use gEfiTtyTermGuid in DevicePathFromText.c in MdePkg to convert >> between a text path to a device path, but >> I don't see how to include TtyTerm.h in DevicePathFromText.c. Right >> now I just have added the extern to DevicePathFromText.c >> to verify functionality. This works, but is obviously not the right >> solution. How should I go about properly getting gEfiTtyTermGuid >> declared in DevicePathFromText.c? >> > > You should not define the GUID yourself, you should add it to the > appropriate .dec file, If you then depend on that package and guid in > a module's .inf, the definition will be emitted implicitly by the > build tools. Thanks Ard. OK, got rid of the bogus Tty.c. I already had the GUID (also) defined in the MdeModulePkg.dec, and the gEfiTtyTermGuid in the [Guid] section of the UefiDevicePathLib.inf. What I am missing is the declaring the dependency of UefiDevicePathLib on the MdeModulePackage, and I don't know how to add that. TerminalDxe isn't under the Library directory, and doesn't have a "LIBRARY_CLASS" in its .inf file, and all the examples of dependencies that I have found seem to rely on that. My somewhat vague understanding of the ED2 build system has hit its limit. Thanks, Roy > > -- > Ard. ------------------------------------------------------------------------------
On Fri, Jun 5, 2015 at 4:39 PM, Laszlo Ersek <lersek@redhat.com> wrote: > On 06/06/15 00:36, Roy Franz wrote: >> On Fri, Jun 5, 2015 at 2:31 AM, Ard Biesheuvel >> <ard.biesheuvel@linaro.org> wrote: >>> On 5 June 2015 at 06:03, Roy Franz <roy.franz@linaro.org> wrote: >>>> On Thu, May 14, 2015 at 1:57 PM, Kinney, Michael D >>>> <michael.d.kinney@intel.com> wrote: >>>>> Roy, >>>>> >>>>> 1) New GUID should be defined in MdeModulePkg/MdeModulePkg.dec >>>>> 2) New include file for new GUID in MdeModulePkg/Include/Guid >>>>> 3) Update TerminalDxe to use this new GUID. >>>>> >>>>> I do not recommend creating a new terminal driver. Please continue as you have which is adding support for this one new terminal type to MdeModulePkg/Universal/Console/TerminalDxe. >>>>> >>>>> Best regards, >>>>> >>>>> Mike >>>>> >>>>> >>>> OK, I have made the above changes, and have it working with one hack >>>> I'm not sure how to properly resolve. >>>> >>>> I now have gEfiTtyTermGuid declared (extern) in >>>> MdeModulePkg/Include/Guid/TtyTerm.h, and defined in >>>> MdeModulePkg/Universal/Console/TerminalDxe/Tty.c. >>>> >>>> I use gEfiTtyTermGuid in DevicePathFromText.c in MdePkg to convert >>>> between a text path to a device path, but >>>> I don't see how to include TtyTerm.h in DevicePathFromText.c. Right >>>> now I just have added the extern to DevicePathFromText.c >>>> to verify functionality. This works, but is obviously not the right >>>> solution. How should I go about properly getting gEfiTtyTermGuid >>>> declared in DevicePathFromText.c? >>>> >>> >>> You should not define the GUID yourself, you should add it to the >>> appropriate .dec file, If you then depend on that package and guid in >>> a module's .inf, the definition will be emitted implicitly by the >>> build tools. >> >> Thanks Ard. >> >> OK, got rid of the bogus Tty.c. I already had the GUID (also) >> defined in the MdeModulePkg.dec, and the gEfiTtyTermGuid >> in the [Guid] section of the UefiDevicePathLib.inf. >> What I am missing is the declaring the dependency of UefiDevicePathLib >> on the MdeModulePackage, and I don't know how to add that. >> TerminalDxe isn't under the Library directory, and doesn't have >> a "LIBRARY_CLASS" in its .inf file, and all the examples of dependencies >> that I have found seem to rely on that. >> My somewhat vague understanding of the ED2 build system has hit its limit. > > In the INF file of any module (library instance, driver module, > application, etc), you have a > > [Packages] > > section, under which you can list *.dec files, with pathnames relative > to the root of the edk2 clone. Once you add a dec file there, you can > reference guids, PCDs, and library classes (headers) declared in that > DEC file. > > Protocol GUIDs and other GUIDs will become available for compilation by > including the appropriate include files (from under Library/, Protocol/, > and Guid/, usually); these relative include file pathnames are then > searched against all modules that you listed under [Packages]. > > For *linking*, you'll also have to list the protocol guids under > [Protocols] and the other guids under [Guids] in the INF file. This will > cause the build system to include *definitions* for these GUIDs in the > auto-generated C files. This way linking too will succeed. > > The [LibraryClasses] section is also there for linking, but it has an > extra level of indirection. (For compilation, see Library/ above.) A > library class ultimately corresponds to a header file only, and it can > have several implementations. [LibraryClasses] therefore lists only the > sets of APIs you'd like to link against, but the actual library > implementation is resolved in the DSC file that you are building. > > The DSC file can resolve a library class to a library instance > - globally, for all modules, > - for types of modules (eg. DXE_DRIVER vs. PEIM vs. UEFI_APPLICATION), > - for individual modules (identified by their INF files). > > (The INF file spec describes this in much more detail, and no doubt much > more correctly; just google it.) > > In addition, library instances (= implementations) can restrict > themselves to some module types. The most common example is that > libraries that write to static variables cannot be generally linked into > PEIMs, because writeable memory becomes available only at some point > during PEI; some PEIMs execute in place from flash. (There's only static > linking; dynamic linking is covered with PPIs (PEIM-to-PEIM interfaces) > and protocols.) > > So a library wanting to write to a static variable would in general > restrict itself to post-PEI module types. Alternatively, its INF file > could list a dependency expression (a depex) on the > "gEfiPeiMemoryDiscoveredPpiGuid" PPI, which gets installed when DRAM > becomes available during PEI. Such a depex would be inherited by any > PEIM that linked against this particular library instance, with the > effect that the PEIM would be first dispatched only after DRAM were > initialized. (Hence allowing the library built into the module to write > to its static variables.) > > Anyway, in the current case, the technical solution would be to add > MdeModulePkg/MdeModulePkg.dec under the [Packages] section in > "MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf". > > However, that's no good, because MdePkg contains definitions, libraries > etc. for industry standards. For example, the DevicePathLib library > instance in question ("UefiDevicePathLib") knows about GUIDs that are > listed in the UEFI specification. So you'd either have to introduce a > cross-package dependency to MdeModulePkg (which is technically feasible, > but purity-wise it would be frowned upon I think, because MdeModulePkg > is reference implementation for a standard, not standard per se), *or* > you'd have to add gEfiTtyTermGuid to MdePkg's dec file, and the > appropriate MdePkg/Include/Guid/... header file). However, the latter > would not be accepted until gEfiTtyTermGuid were actually *standard*. > > So what can you do, if neither the cross-pkg dependency nor the direct > MdePkg modification is appropriate? Two options: > - Try to standardize the GUID with the USWG. Good luck! :) > - Fork the UefiDevicePathLib library instance to some other (less > central) package, and modify it there. This sucks because any updates > to the standard location would have to be cross-ported, going forward. > > The edk2 build system is actually very flexible, it's just that the > terminal stuff has always proven untouchable (to me at least). Note > though that forking a library instance (or a module for that matter) is > the *norm* for proprietary vendors, which is what edk2 (and the UEFI > spec itself) are optimized for. (Eg. the protocols are ABIs, not APIs.) > > To summarize: you're facing this problem because the devpath-to-text > conversion library instance under MdePkg is tightly coupled with an > industry standard (the UEFI spec), whereas the driver modules under > MdeModulePkg are just "independent implementations" (that Intel cares > about very much though). If you want to add non-standard extensions to > the devpath-to-text conversion library, you might have to fork it (or > standardize the extension). > > Sorry if I misunderstood the situation; hopefully others can correct me > then (or anyway). Thank you - that was very helpful, and I think you do understand the situation. I had some inklings of some of problems you highlighted above, but not the full ramifications. So, the link problem I am having is in the device path conversion code (DevicePathTo/FromText.c), so is there a way to avoid this by just not supporting nice text device paths? From my (again limited) understanding of device paths, I could do something like: VenHw(D3987D4B-971A-435F-8CAF-4967EB627241) instead of VenTtyTerm(), which would directly encode the GUID, and hence avoid the dependency problems I'm having. It will make for a more ugly .dsc file, but that's what comments are for :) Would this work? Thanks, Roy > > Thanks > Laszlo ------------------------------------------------------------------------------
diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c index 51492f3..70ec370 100644 --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c @@ -1561,8 +1561,14 @@ UnicodeToEfiKey ( } if (UnicodeChar == DEL) { - Key.ScanCode = SCAN_DELETE; - Key.UnicodeChar = 0; + if (PcgGetBool(TerminalTreatAsciiDelAsBackspace) {^M + Key.ScanCode = SCAN_NULL;^M + Key.UnicodeChar = CHAR_BACKSPACE;^M + }^M + else {^M + Key.ScanCode = SCAN_DELETE;^M + Key.UnicodeChar = 0;^M + }^M > > Mike